Snap components #2166
Merged
Snap components #2166
Conversation
This file contains 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
Specify nvidia 510 to for dry-run drivers matching purposes. Co-authored-by: Dan Bungert <[email protected]>
dbungert
force-pushed
the
snap-components
branch
from
0e12b84 to
a0f1a0e
Compare
|
FFE: LP:#2099950 |
Co-authored-by: Dan Bungert <[email protected]>
dbungert
force-pushed
the
snap-components
branch
from
a0f1a0e to
9212dc5
Compare
Chris-Peterson444
approved these changes
Feb 25, 2025
Comment on lines
+245
to
+249
| def snap_of_type(self, typ: ModelSnapType) -> Optional[ModelSnap]: | ||
| for snap in self.snaps: | ||
| if snap.type == typ: | ||
| return snap | ||
| return None |
There was a problem hiding this comment.
nit: this only gets the first of the type in the list. From the name and usage it seems expected that there's only one of this type, which is true for the kernel (could we ever have more?), but it wouldn't work for say typ=ModelSnapType.APP where we already have multiple.
I think instead we want something like:
Suggested change
| def snap_of_type(self, typ: ModelSnapType) -> Optional[ModelSnap]: | |
| for snap in self.snaps: | |
| if snap.type == typ: | |
| return snap | |
| return None | |
| def snaps_of_type(self, typ: ModelSnapType) -> List[ModelSnap]: | |
| return [snap for snap in self.snaps if snap.type == typ] |
and have the caller make the assertion about the number of them?
I wouldn't say it's blocking and think it's fine to fix this in a follow up though.
ogayot
reviewed
Feb 25, 2025
If we had the scenario of multiple recommended drivers, and matching component versions, prefer the newer ones.
dbungert
force-pushed
the
snap-components
branch
from
9212dc5 to
ceea355
Compare
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.
No description provided.