-
Notifications
You must be signed in to change notification settings - Fork 333
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
python-setup: Don't allow Poetry to make venv in project #1419
Conversation
I mostly verified this works on my local machine, but did add a sample `poetry.toml` to the tests, so it can be verified from looking at the logs 🤷
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 think we need to update the README for this change 😄
As for the CHANGELOG — do we expect the change to affect many users? If the venv was previously considered user-written code, and now isn't, it shouldn't affect any user's results (unless the venv itself had alerts), right?
Many? that's not my expectation, but it's a gut feeling and not based on hard data.
spot on. I think there's a fair chance there would have been spurious alerts found within the in-project venv though. Altogether, I have time defending not making adding anything to the changelog, so let me just do that 👍 |
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 updating!
@@ -3,6 +3,7 @@ | |||
## [UNRELEASED] | |||
|
|||
- Add a step that tries to upload a SARIF file for the workflow run when that workflow run fails. This will help better surface failed code scanning workflow runs. [#1393](https://github.com/github/codeql-action/pull/1393) | |||
- Python automatic dependency installation will no longer consider dependecy code installed in venv as user-written, for projects using Poetry that specify `virtualenvs.in-project = true` in their `poetry.toml`. [#1419](https://github.com/github/codeql-action/pull/1419). |
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.
- Python automatic dependency installation will no longer consider dependecy code installed in venv as user-written, for projects using Poetry that specify `virtualenvs.in-project = true` in their `poetry.toml`. [#1419](https://github.com/github/codeql-action/pull/1419). | |
- Python automatic dependency installation will no longer consider dependency code installed in `venv` as user-written, for projects using Poetry that specify `virtualenvs.in-project = true` in their `poetry.toml`. [#1419](https://github.com/github/codeql-action/pull/1419). |
Quick typo suggestion
As highlighted in the code comments, Projects that specify
in-project = true
in their poetry.toml would get the venv created inside the repo directory, which would cause CodeQL to consider it as user-written code. We don't want this to happen.Merge / deployment checklist
I have not updated changelog or readme since I thought this was a small enough change, but let me know if you think that woudl be needed.