Fixes #1: Add option to build with poetry#2
Conversation
Add option to install dependencies with poetry in the action. Signed-off-by: Abishek V Ashok <abishekvashok@gmail.com>
facebook-github-bot
added
the
CLA Signed
arthaud
left a comment
arthaud
left a comment
There was a problem hiding this comment.
Q: how does poetry infer dependencies? It must use requirements.txt right? What's the benefit of poetry then?
| requirements-path: | ||
| description: Path to requirements file, relative to `repo-directory`, to look for dependencies. Default will look for requirements.txt in the root of repo-directory | ||
| required: true | ||
| required: false |
There was a problem hiding this comment.
What does this do? I would assume we "require" requirements.txt
There was a problem hiding this comment.
@arthaud Poetry uses dependencies in pyproject.toml, it also supports .lock files like yarn and npm. So, we don't hav eto have a requirements.txt file.
We could have a check for poetry before poetry install though, like something we do for the requirements file (check if the file exists, in this case pyproject.toml)
|
It looks like there are CI errors, could you look into those? |
The condition for poetry used to evaluate to true always. Fix this by evaluating against `true`. Signed-off-by: Abishek V Ashok <abishekvashok@gmail.com>
|
@arthaud I think it is fixed now. Sorry for messing up earlier. You might want to re-approve for tests. Also consider, removing needing of approval for workflows - I don't think we are anywhere near for Github actions in this repo :) |
|
Well, it looks like there are failures on master now, could you look? |
|
@arthaud sure :) Do we have past logs stored elsewhere? The past github action logs have expired. I see the primary cause being pysa unable to detect issues as we aren't including the modules in the search path. The action that fails detects 0 issues while the one succeeding has found 8. Thus sapp creates an empty json file which is not happily accepted by the github sariff parser. The only change between the actions is that one uses .pyre_configuration and the other doesn't. I believe adding LD_LIBRARY_PATH to search_path will fix this (make pysa able to find the issues.) I will also try to make a pr for sapp to atleast output an empty file confronting to the sariff specs if there are no vulnerabilities :) Also for the logs, consider enabling monthly run of the github workflows in this repository. The expiration time for logs seem to be 90 days (https://docs.github.com/en/actions/using-workflows/storing-workflow-data-as-artifacts), so we can have at least 2 logs for each workflow available at any time. |
|
So took a bit of time to dig into this. Including every module in the path was not the solution but that the submodule is too old (snapshot is from a commit in 2022, pysa and pyre-check have been both changed a lot since then) I made a PR to fix this and stop relying on submodules/snapshots: #3 |
3 participants
Add option to install dependencies with poetry in the action.
Signed-off-by: Abishek V Ashok abishekvashok@gmail.com
Poetry docs:
https://python-poetry.org/docs/#installing-with-pipx
https://python-poetry.org/docs/basic-usage/#installing-dependencies-only
Why
--no-root: poetry installs projects as editable by default with--no-rootwe get the same behavior as pip install.Fixes #1