Install Python tools on self-hosted runners #618
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aeisenberg
reviewed
Jul 13, 2021
src/init.ts
Outdated
| @@ -237,7 +221,10 @@ export async function installPythonDeps(codeql: CodeQL, logger: Logger) { | |||
| } catch (e) { | |||
| logger.endGroup(); | |||
| logger.warning( | |||
| "We were unable to install your python dependencies. You can call this action with 'setup-python-dependencies: false' to disable this process." | |||
| "We were unable to automatically install your python dependencies. Please make sure any necessary " + | |||
There was a problem hiding this comment.
Do you also want to log the error here?
There was a problem hiding this comment.
Thanks, done.
edoardopirovano
force-pushed
the
python-deps
branch
from
ee46daf to
cc14f29
Compare
aeisenberg
approved these changes
Jul 13, 2021
|
__ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR changes the
initstep to also install the Python packages that the Action requires when running on self-hosted runners, as we already do on GitHub ones. The rationale given in #155 for not doing this was that "we don't want our cli to be messing around with things in the system". On the one hand, this seems reasonable. However, I think we should try to make a best attempt to succeed with our analysis and in my opinion this means installing tools we require.Notably, we are already messing around with things that are installed since the later step will install an arbitrary number of packages that are dependencies of the code so that we can analyse it. If we are happy to do this, I see no reason why we shouldn't also be happy to install the four Python packages that we need.
This PR was prompted by #615, where the user's workflow failed due to this (and one other issue).
Merge / deployment checklist