-
Notifications
You must be signed in to change notification settings - Fork 70
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
Document workaround for the broken snactor #799
Conversation
Your git-forge project |
Thank you for contributing to the Leapp project!Please note that every PR needs to comply with the Leapp Guidelines and must pass all tests in order to be mergable.
To launch regression testing public members of oamg organization can leave the following comment:
Please open ticket in case you experience technical problem with the CI. (RH internal only) Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please consider rerunning the CI by commenting leapp-ci build (might require several comments). If the problem persists, contact leapp-infra. |
@@ -328,6 +328,17 @@ data is stored in the database for the current session: | |||
snactor run --save-output IPUWorkflowConfig | |||
``` | |||
|
|||
Because of a bug in leapp, this does not work and gives the following error: |
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.
Technically it's not a bug or something that can be fixed for good, as snactor is a development tool and it is not being run on the machine-to-be-upgraded, so it has no knowledge of what system leapp is planned to be run for and thus needs this information to be passed this way or another. Setting LEAPP_UPGRADE_PATH_TARGET_RELEASE
and LEAPP_UPGRADE_PATH_FLAVOR
to the values you mentioned is the way to say "Run leapp actors as you would for a 7to8 upgrade on a typical rhel system".
Could you please rephrase the description a bit? Maybe something like After leapp started supporting several upgrade paths snactor requires target system and upgrade flavor specification for a correct execution. To get around the possible leapp ModelViolationError ... please set the following environment variables prior to running snactor
.
And thanks a lot for the patch, that's definitely an awesome addition to the dev docs!
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.
Thanks for the feedback. I reworded the changed based on your suggestion.
Giving this another though, would it make sense to have this information somewhere inside of the snactor
, when it is executed without these variables, reporting that it might not work for all the workflow related tasks, at least as a warning? Or even make it mandatory for execution of snactor
? Or provide some user-friendly commandline switches for snactor
to set this for you?
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.
You are definitely right, we should add this information to snactor. I'll create an issue for that.
prefer having this documented instead answering everyone questions why it does not work when it is in documentation. At least until it is fixed.
Your git-forge project |
This PR has been linked in issue tracker (#OAMG-7725). |
Co-authored-by: Petr Stodůlka <xstodu05@gmail.com>
Your git-forge project |
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.
Looks perfect now, thanks!
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.
Same as Ina. Thanks!
prefer having this documented instead answering everyone questions why it does not work when it is in documentation. At least until it is fixed.