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

Use uv to manage dependencies #268

Merged
merged 4 commits into from
Mar 27, 2024
Merged

Conversation

fyliu
Copy link
Member

@fyliu fyliu commented Mar 13, 2024

Fixes #178
Fixes #183

What changes did you make?

  • Added missing doc item for ERD generating script
  • Set up uv to manage dependencies + docs
  • Replace pip with uv for faster installs
  • Set up docker buildkit cache mounts for faster image rebuilds + docs

Why did you make the changes (we will use this info to test)?

  • We needed a way to update dependencies

How to test

  • verify the update script works ./scripts/update-dependencies.sh
  • verify the buildrun script works ./scripts/buildrun.sh
  • verify the erd script works ./scripts/erd.sh
  • verify all tests still pass ./scripts/test.sh

@fyliu
Copy link
Member Author

fyliu commented Mar 13, 2024

Rebased to latest main and force pushed

@fyliu fyliu changed the title uv for managing dependencies Use uv to manage dependencies Mar 14, 2024
@fyliu fyliu closed this Mar 14, 2024
@fyliu
Copy link
Member Author

fyliu commented Mar 14, 2024

Found a problem when running where nc is required. Will undo those changes and re-evaluate the apt requirements later.

@fyliu fyliu reopened this Mar 15, 2024
@fyliu fyliu marked this pull request as draft March 15, 2024 00:17
@fyliu
Copy link
Member Author

fyliu commented Mar 15, 2024

restored the missing apt dependencies and force pushed

@fyliu fyliu marked this pull request as ready for review March 15, 2024 23:49
@freaky4wrld freaky4wrld self-requested a review March 19, 2024 05:15
Copy link
Member

@freaky4wrld freaky4wrld left a comment

Choose a reason for hiding this comment

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

@fyliu these are my observations while testing

  • ./scripts/update-dependencies.sh can't be found in the branch

  • ./scripts/buildrun.sh works great, as desired uv gets downloaded and we are using the djaongo version of 4.2.11

  • ./scripts.test.sh also works good as well

  • ./scripts/erd.sh I'm not sure what it does except, it should show and ERD diagram, but all I got as an output is

    >> ./scripts/erd.sh      
    + docker-compose exec web python manage.py graph_models --pydot -a -g -o erd.png
    

@freaky4wrld freaky4wrld self-requested a review March 20, 2024 10:59
Copy link
Member

@freaky4wrld freaky4wrld left a comment

Choose a reason for hiding this comment

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

@fyliu Thanks for working on the changes, the script update-dependencies.sh is available and on execution make changes to requirement.txt by updating the versions.

I'm still not sure on the ./scripts/erd.sh, can't find an end.png

@fyliu
Copy link
Member Author

fyliu commented Mar 21, 2024

I'm still not sure on the ./scripts/erd.sh, can't find an end.png

@freaky4wrld It should be in app/erd.png

❯ file app/erd.png
app/erd.png: PNG image data, 5113 x 1379, 8-bit/color RGBA, non-interlaced

I also amended the erd commit with the location of the generated image.

Copy link
Member

@freaky4wrld freaky4wrld left a comment

Choose a reason for hiding this comment

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

@fyliu thanks for making the changes, here is my observations as per the new changes done:

  • update-dependencies.sh produces updated version of tools in requirements.txt
  • build run.sh runs successfully
  • erd.sh produces erd.png in the app directory
  • test.sh successfully checks for the test provided

PR approved!!!! Thanks for your contribution, really appreciate all your efforts :)

@fyliu fyliu merged commit 91f0f5e into hackforla:main Mar 27, 2024
1 check passed
@fyliu fyliu mentioned this pull request Apr 7, 2024
@fyliu fyliu deleted the 178-uv-dependencies branch June 5, 2024 19:04
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.

Update dependency versions Use uv to manage dependencies
2 participants