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.
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
Add runtime instructions #680
base: main
Are you sure you want to change the base?
Add runtime instructions #680
Changes from all commits
fa5c241
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
so is this supposed to be run inside the venv or outside? if it's inside the venv, you don't need to install python3-pip anymore, it is provided by the venv.
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.
True only if running in a venv. I phrased the previous block as optional so in cases where people run this outside a venv (for whatever reasons) it would still be needed.
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.
why not simply install python3-pip (or python311-pip) instead? seems simpler than this process imho.
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.
But python3-pip is installed just in the line before. This is there to update pip, which we do, but I'm not sure if that's really needed.
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.
It's not needed. but you should be installing python311 instead which is 3.11 rather than 3.6.
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.
This also needs to work on older versions, so this is the most generic version that will work at least on Leap and Tumbleweed.
If we advance to specifying python311 it will not work on Leap and require us to update the documentation every time there is a Leap release or Tumbleweed updates the major python version in use.
For the sake of making the document age better, I would stay with the generic version.
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.
Maybe add a
--user
flag here?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.
I think we should advertise installation of packages, or if that is unwanted for some reason, advertise the use of venvs instead of pip install globally (or as user which is also not possible on tumbleweed)
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.
I do like to advertise the use of venvs more than using the
--user
flag, so I added instructions to use a virtualenv.By using the
--user
flag we need to explain that this will only work outside of virtualenvs and it's also not what we are using in our test flow in openQA, so I think that's not what we should document.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.
actually I'd prefer if we wouldn't use the pip method in openQA either from a supply chain security perspective..
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.
Let's make this a meeting point on Tuesday, that requires more more engineering work especially on older versions (12-SP5) but if we want to do this, it should be possible.
For now I suggest we decouple this initiative from this PR, because that's not easy to solve and not worth waiting for IMHO.