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
Fix linux pack actions For CentOS 8 #5035
Fix linux pack actions For CentOS 8 #5035
Changes from 5 commits
6eba49d
f38ccda
9139fc0
473a941
7be79f6
50f2774
fb4d4dc
cda720d
88222d1
cbf523c
1c12531
b25cc4f
b756326
749c8bf
a82b1be
f8f7951
cb4f9bb
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.
Looks like this is the only place with the hardcoded
python2
now.Is it possible to do something with that so we don't rely on a specific version?
Besides of that, looks good.
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.
Yeah, any reason why we need to use
python2
here? I believe that line should also work fine with Python 3 (aka just usingpython
system binary - should work if it's either python 2 or python 3).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 also wonder if there is a better way which doesn't rely on
eval()
which can be unsafe, especially in this context when we are passing 3rd party data to it.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 don't see any issues regarding py3 and it should work as well (I can put that on my list again).
Let's also tackle the
eval
. The safe alternative without a bunch of JSON processing on stdin is ast.literal_eval: https://docs.python.org/3/library/ast.html#ast.literal_evalWould this be fine for everyone? Last resort would be some manual parsing & processing with the JSON library.
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.
ast.literal_eval
is awesome (and quite safe)! go for it.This comment was marked as outdated.
Sorry, something went wrong.