Skip to content

Conversation

@ArangoGutierrez
Copy link
Collaborator

No description provided.

@ArangoGutierrez ArangoGutierrez self-assigned this Oct 23, 2024
@NVIDIA NVIDIA deleted a comment from ArangoGutierrez Oct 23, 2024
Copy link
Collaborator Author

@ArangoGutierrez ArangoGutierrez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the Help @klueska

defer func() {
cancel()
if err := imexManager.Stop(); err != nil {
klog.Errorf("error stopping IMEX manager: %v", err)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
klog.Errorf("error stopping IMEX manager: %v", err)
klog.Errorf("Error stopping IMEX manager: %v", err)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh I forgot linters don't like capitalized error messages

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted this change, to comply with the linter

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WONT-DO

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  Running [/home/runner/golangci-lint-1.61.0-linux-amd64/golangci-lint run  -v --timeout 5m] in [/home/runner/work/k8s-dra-driver/k8s-dra-driver] ...
  Error: cmd/nvidia-dra-controller/imex.go:309:11: ST1005: error strings should not be capitalized (stylecheck)
  			return fmt.Errorf("Error deleting resource slice %s: %w", rs.Name, err)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe your golangci-lint or go versions are different from the one we have on CI

Copy link
Collaborator

@klueska klueska Oct 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is not the correct line you are changing. That one should be lower case because it is a return statement. The line I'm requesting to change is the klog line which should be uppercase.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, I understood my error, I was looking into the wrong line

@ArangoGutierrez ArangoGutierrez force-pushed the 107/131 branch 2 times, most recently from 6beddd6 to 0bc9bb6 Compare October 23, 2024 14:09
@ArangoGutierrez
Copy link
Collaborator Author

Ready for a last review

@klueska
Copy link
Collaborator

klueska commented Oct 24, 2024

Can you squash these into one commit

@ArangoGutierrez
Copy link
Collaborator Author

Can you squash these into one commit

done

@klueska
Copy link
Collaborator

klueska commented Oct 24, 2024

Once you have addressed https://github.com/NVIDIA/k8s-dra-driver/pull/186/files#r1812822764 I think we ae good to go.

Add more synchronization around the shutdown of the IMEX manager

Signed-off-by: Kevin Klues <[email protected]>
Signed-off-by: Carlos Eduardo Arango Gutierrez <[email protected]>
@ArangoGutierrez ArangoGutierrez merged commit 1ef60aa into NVIDIA:main Oct 24, 2024
6 checks passed
ArangoGutierrez added a commit to ArangoGutierrez/k8s-dra-driver that referenced this pull request Oct 25, 2024
Clean up resourceSlices during exit
ArangoGutierrez added a commit to ArangoGutierrez/k8s-dra-driver that referenced this pull request Oct 25, 2024
Clean up resourceSlices during exit
ArangoGutierrez added a commit to ArangoGutierrez/k8s-dra-driver that referenced this pull request Oct 25, 2024
Clean up resourceSlices during exit
@klueska klueska added this to the v25.3.0 milestone Aug 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

2 participants