feat: CollectDiagnosticData #2
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tmcginnis#419) * common: Extract common http patch/post code into separate functions * Refactor VirtualMedia.InsertMedia to call InsertMediaConfig No need for similar implementations when InsertMedia is basically a specialization of InsertMediaConfig. * virtualmedia: Return Tasks if present in InsertMedia http response
* Bump golangci/golangci-lint-action from 6 to 8 Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 6 to 8. - [Release notes](https://github.com/golangci/golangci-lint-action/releases) - [Commits](golangci/golangci-lint-action@v6...v8) --- updated-dependencies: - dependency-name: golangci/golangci-lint-action dependency-version: '8' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * Update golangci-lint to v2.1.6 Updates the linter and fixes the few issues found. Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com> --------- Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Sean McGinnis <sean.mcginnis@gmail.com>
…mcginnis#424) This commit *adds* more SMC entity field names that should be allowed for read/write. This started as matching the field names provided in https://www.supermicro.com/manuals/other/redfish-ref-guide-html/Content/general-content/bmc-configuration-examples.htm but then continued to follow the JSON field names that were found higher in each file as that matched what a new SMC chassis expected. The previous field names were left as read/write field names in case some of the older chassis BMCs expect those names. The NTP and Syslog file changes have been confirmed to fix bugs that we've seen on our end. All of the changes are additions rather than updates/renames, so this should break nothing.
Update payload logic was not aware of things like ",omitempty". This would cause an invalid field name in the PATCH payload to update objects with optional fields. This updates the handling to strip out this string from the JSON property name. Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com>
iamsli
previously approved these changes
Jun 16, 2025
jplimack
reviewed
Jun 16, 2025
update_gofish.sh
Outdated
| @@ -37,6 +45,6 @@ else | |||
| fi | |||
|
|
|||
| # Merge back into main | |||
| git checkout main | |||
| git checkout "$PR_BRANCH_NAME" | |||
There was a problem hiding this comment.
which remote is this checking out? this should be getting a fresh copy of upstream/main and then applying our patch on top of that, then pushing to internal/PR_BRANCH_NAME.
There was a problem hiding this comment.
is it wrong as-is now?
so:
- line 30: checkout main (should be origin AKA internal main)
- 31: create the PR branch locally
- fetch upstream and rebase it onto the PR branch
- still on PR branch
- 37: create a new apply branch to apply our patch
- 48: come back to our local PR branch (already rebased onto upstream)
- merge the patch back in again
the logic in the script was a bit confusing to me, but AFAIK, the only thing I changed was instead of rebasing origin/main we rebase $PR_BRANCH and then push that up to origin/$PR_BRANCH
There was a problem hiding this comment.
i think thats right. perhaps if we renamed the remotes to upstream and internal, it might make its more clear/explicit
jplimack
reviewed
Jun 16, 2025
05ebad9 to
fff6384
Compare
jplimack
previously approved these changes
Jun 17, 2025
The merge-base changed after approval.
jplimack
approved these changes
Jun 17, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
add support for CollectDiagnosticData
fixes https://coreweave.atlassian.net/browse/METALDEV-937