Skip to content
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

⬆️ [maykinmedia/open-api-framework#93] Security updates #216

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

stevenbal
Copy link
Contributor

Fixes maykinmedia/open-api-framework#93 partially
Fixes maykinmedia/open-api-framework#81 partially

Changes

  • Replace pip-compile with uv
  • Security updates

set -ex

command -v uv || (echo "uv not found on PATH. Install it https://astral.sh/uv" >&2 && exit 1)

cwd="${PWD}"
Copy link
Contributor

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 cwd if we use toplevel in the pip-compile commands (as git rev-parse --show-toplevel returns an absolute path), for example

uv pip-compile \
   --output-file "$toplevel/requirements/base.txt" \
    "$@" \
    "$toplevel/requirements/base.in"

You could also call it something like root_dir to make the variables purpose more descriptive.

Comment on lines +1 to +2
-c ci.txt
-r ci.txt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be done in the compile_dependencies script?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is like this in the default project and I also think its more correct since if you use pip install -r requirements/dev.in, it will install everything needed for dev vs before where it would just install those 5 libraries.

Though if you just want to install them, its less convenient but I cant think of a situation where I would.

Comment on lines +1 to +2
-c base.txt
-r base.txt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment above

Comment on lines +1 to +2
-c base.txt
-r base.txt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also applies here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bump third party library versions to fix security issues Use UV in the compile_dependencies scripts
3 participants