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

Emit a warning when and/or groups are combined with HW requirements #3582

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Emit a warning when and/or groups are combined with HW requirements #3582

wants to merge 2 commits into from
+73 −0

Conversation

happz
Copy link
Collaborator

@happz happz commented Mar 6, 2025 •
edited
Loading

Pull Request Checklist

  • implement the feature
  • write the documentation
  • extend the test coverage
  • update the specification
  • modify the json schema

@happz happz added documentation Improvements or additions to documentation area | hardware Implementation of hardware requirements code | schema Schema used for validating config files labels Mar 6, 2025
@happz happz mentioned this pull request Mar 6, 2025
Open
@happz happz linked an issue Mar 6, 2025 that may be closed by this pull request
or/and will override other hardware constraints in Beaker job XML #3273
Open
skycastlelily
skycastlelily approved these changes Mar 7, 2025
View reviewed changes
Copy link
Collaborator

@skycastlelily skycastlelily left a comment

Choose a reason for hiding this comment

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

LGTM^^

psss
psss requested changes Mar 7, 2025
View reviewed changes
Copy link
Collaborator

@psss psss left a comment

Choose a reason for hiding this comment

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

Nice, thanks for improving this! Seems that valid hardware in the test also complains. Plus warnign typo in the commit summary. Could you please update the commit message so that the summary is not split in the middle of a word? Thanks.

tests/provision/hardware/test.sh
rlAssertGrep "is not valid under any of the given schemas" $rlRun_LOG

rlRun -s "tmt -vv plan show /plan/or-does-not-combine"
rlAssertGrep "is not valid under any of the given schemas" $rlRun_LOG
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would be good to include at least one positive test as well. Here, the invalid under any message is given even for a valid hardward config. Seems we need to explicitly mention how: artemis or update the tmt/schemas/provision/virtual.yaml schema to reference hardware as well. Probably the second would be better as virtual should already accept many hardware keys?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added a positive test, and the fix for virtual is indeed needed, #3585

@happz happz force-pushed the emit-warning-on-andor-combinations branch from 9465352 to bf12610 Compare March 7, 2025 13:53
@happz happz changed the title Emit a warnign when and/or HW requirement groups are combined with ot… Emit a warning when and/or groups are combined with HW requirements Mar 7, 2025
@happz
Copy link
Collaborator Author

happz commented Mar 10, 2025

/packit build

@happz happz force-pushed the emit-warning-on-andor-combinations branch from bf12610 to 321b19b Compare March 12, 2025 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@psss psss psss requested changes

@skycastlelily skycastlelily skycastlelily approved these changes

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

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

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

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

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
area | hardware Implementation of hardware requirements code | schema Schema used for validating config files documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

or/and will override other hardware constraints in Beaker job XML
3 participants
@happz @psss @skycastlelily