Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Simplify parsing of HW requirements and add missing ones #2928

Merged
merged 1 commit into from
Jun 11, 2024
Merged

Simplify parsing of HW requirements and add missing ones #2928

merged 1 commit into from
Jun 11, 2024

Conversation

happz
Copy link
Collaborator

@happz happz commented May 13, 2024

It turned out some requirements were left out and nothing was parsing them. The "maximal" unit test was incorrect and did not report this.

The patch

  • adds parsing for missing requirement groups (gpu, device),
  • several individual requirements were also missing,
  • simplifies parsing of groups of constraints, providing helpers to avoid repetition,
  • and fixes the test guarding this.

Pull Request Checklist

  • implement the feature
  • extend the test coverage

@happz happz added bug Something isn't working area | hardware Implementation of hardware requirements test coverage Improvements or additions to test coverage of tmt itself labels May 13, 2024
@happz happz added this to the 1.34 milestone May 13, 2024
@happz happz added the status | blocked The merging of PR is blocked on some other issue label May 13, 2024
@happz
Copy link
Collaborator Author

happz commented May 13, 2024

Blocked on #2924

@juk
Copy link
Collaborator

juk commented May 15, 2024 •
edited
Loading

@happz thank for an update! Would you consider adding requirement group "device" as described in https://tmt.readthedocs.io/en/stable/spec/hardware.html#device ? In some cases like Driver Update Program we do not classify devices as gpu/network/etc but need to find systems by device's ID only

@happz
Copy link
Collaborator Author

happz commented May 15, 2024

@happz thank for an update! Would you consider adding requirement group "device" as described in https://tmt.readthedocs.io/en/stable/spec/hardware.html#device ? In some cases like Driver Update Program we do not classify devices as gpu/network/etc but need to find systems by device's ID only

device should be one of the refactored and completed requirements, see https://github.com/teemtee/tmt/pull/2928/files#diff-1f37ee00a19e467a4f192fb6c06f6f3cce3b9e460896ed5397a89054bf58d085R1371

Note that this patch focuses on shared library part of HW requirements - the implementation for Beaker provisioning will follow soon.

@juk
Copy link
Collaborator

juk commented May 15, 2024

device should be one of the refactored and completed requirements, see https://github.com/teemtee/tmt/pull/2928/files#diff-1f37ee00a19e467a4f192fb6c06f6f3cce3b9e460896ed5397a89054bf58d085R1371

Oh sorry, missed it. Thank you!

@happz happz force-pushed the hw-parsing-better-code branch 2 times, most recently from 8e49562 to 3baff21 Compare May 27, 2024 12:51
@happz happz removed the status | blocked The merging of PR is blocked on some other issue label May 27, 2024
@happz happz added the ci | full test Pull request is ready for the full test execution label May 31, 2024
@happz
Simplify parsing of HW requirements and add missing ones
68baed2 
It turned out some requirements were left out and nothing was parsing
them. The "maximal" unit test was incorrect and did not report this.

The patch

* adds parsing for missing requirement groups (`gpu`, `device`),
* several individual requirements were also missing,
* simplifies parsing of groups of constraints, providing helpers to
  avoid repetition,
* and fixes the test guarding this.
thrix
thrix approved these changes Jun 11, 2024
View reviewed changes
Copy link
Collaborator

@thrix thrix left a comment

Choose a reason for hiding this comment

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

LGTM

@happz happz added the status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish. label Jun 11, 2024
@happz
Copy link
Collaborator Author

happz commented Jun 11, 2024

Unrelated failures -> merging.

@happz happz merged commit e5d9b95 into main Jun 11, 2024
17 of 19 checks passed
@happz happz deleted the hw-parsing-better-code branch June 11, 2024 11:37
falconizmi pushed a commit that referenced this pull request Jun 17, 2024
@happz
Simplify parsing of HW requirements and add missing ones (#2928)
e606a83 
It turned out some requirements were left out and nothing was parsing
them. The "maximal" unit test was incorrect and did not report this.

The patch

* adds parsing for missing requirement groups (`gpu`, `device`),
* several individual requirements were also missing,
* simplifies parsing of groups of constraints, providing helpers to
  avoid repetition,
* and fixes the test guarding this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thrix thrix thrix approved these changes

@therazix therazix therazix approved these changes

@psss psss Awaiting requested review from psss psss is a code owner

@lukaszachy lukaszachy Awaiting requested review from lukaszachy lukaszachy is a code owner

@janhavlin janhavlin Awaiting requested review from janhavlin janhavlin is a code owner

Assignees
No one assigned
Labels
area | hardware Implementation of hardware requirements bug Something isn't working ci | full test Pull request is ready for the full test execution status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish. test coverage Improvements or additions to test coverage of tmt itself
Projects
None yet
Milestone
1.34
Development

Successfully merging this pull request may close these issues.
4 participants
@happz @juk @thrix @therazix