-
Notifications
You must be signed in to change notification settings - Fork 18
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.
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
Implement consistent step blocker system #1171
Conversation
8013881
to
01b9f97
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1171 +/- ##
==========================================
+ Coverage 72.99% 73.09% +0.09%
==========================================
Files 94 95 +1
Lines 6532 6578 +46
==========================================
+ Hits 4768 4808 +40
- Misses 1764 1770 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@cpignedoli this should hopefully resolve the issue you and I ran into. It should also improve UX quite a bit by allowing us to more easily prevent invalid runs. There is also the issue of @superstar54 (#1148) that will further improve UX! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @edan-bainglass, LGTM! the only additonal comment that you asked to write here is about triggering a refresh resources once the qe installation is finished (otherwise there is the risk to not see the new code, in step 3)
Thanks!
@mikibonacci can you check once more, see that indeed code selectors are refreshed and enabled on completed setup? 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This PR includes the following:
common/wizard.py
QeWizardStep
and corresponding model are extended by a confirmable and dependent (and both) flavorsHasBlockers
mixin is provided to encapsulate the relevant traits and functionalityConfirmable
andHasBlockers
mixinsHasBlockers
mixin is added toSettingsModel
(panels in steps 2 and 3 can now block)The design is flexible, modular, and extensible. Following the already-implemented use case of step 1, blockers could by introduced in step 2 covering invalid parameter combinations. Similarly in step 3, in addition to the QE setup blockers, additional blockers could be introduced covering invalid resource selection.
Thank you to @superstar54 and @mikibonacci for feedback and input 🙏
Resolves #1151
Resolves #1152