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 in CI and to build images #2403

Merged
merged 15 commits into from
May 10, 2024
Merged

Use uv in CI and to build images #2403

merged 15 commits into from
May 10, 2024

Conversation

eapolinario
Copy link
Collaborator

@eapolinario eapolinario commented May 9, 2024

Tracking issue

flyteorg/flyte#5346

Why are the changes needed?

As described in the issue, let's adopt uv as the package installer for flytekit.

What changes were proposed in this pull request?

Use uv in a few places:

  1. when setting up a dev environment via the new make target setup-uv. This assumes that a virtual environment (e.g. uv venv) is already setup prior to calling this. This keeps the tradition of not being too prescriptive about venvs.
    i. we can make this the default once we're satisfied with it. This will require a slight change to the docs.
  2. the default flytekit image is now built using uv
    i. Notice the use of --system to ensure the packages are installed in the global environment.
  3. the dev flyte image is also built using uv
  4. We should probably also build the agent image with uv.

How was this patch tested?

Tested CI locally.
I built and used the other two images in local sandbox.

Setup process

Screenshots

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Related PRs

Docs link

Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
@eapolinario
Copy link
Collaborator Author

This will probably break on windows.

Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Copy link

codecov bot commented May 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.90%. Comparing base (d2c7ddc) to head (dd7f820).
Report is 34 commits behind head on master.

❗ Current head dd7f820 differs from pull request most recent head dd9952e. Consider uploading reports for the commit dd9952e to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2403      +/-   ##
==========================================
+ Coverage   72.27%   75.90%   +3.62%     
==========================================
  Files         181      181              
  Lines       18302    18393      +91     
  Branches     3576     3601      +25     
==========================================
+ Hits        13228    13961     +733     
+ Misses       4460     3831     -629     
+ Partials      614      601      -13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@eapolinario
Copy link
Collaborator Author

dbt failures are unrelated. Being tracked in flyteorg/flyte#5351.

@eapolinario eapolinario merged commit 0d4981b into master May 10, 2024
45 of 46 checks passed
fiedlerNr9 pushed a commit that referenced this pull request Jul 25, 2024
* Use uv in CI and to build images

Signed-off-by: Eduardo Apolinario <[email protected]>

* Source venv and remove extraneous -y

Signed-off-by: Eduardo Apolinario <[email protected]>

* Use format that uv understands

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix lint errors

Signed-off-by: Eduardo Apolinario <[email protected]>

* Test sourcing the venv in hypothesis job

Signed-off-by: Eduardo Apolinario <[email protected]>

* Source everywhere

Signed-off-by: Eduardo Apolinario <[email protected]>

* Remove redundant shellcheck

Signed-off-by: Eduardo Apolinario <[email protected]>

* Add activate-uv-venv target

Signed-off-by: Eduardo Apolinario <[email protected]>

* Install uv in the system python

Signed-off-by: Eduardo Apolinario <[email protected]>

* Add --system

Signed-off-by: Eduardo Apolinario <[email protected]>

* Install plugin dev-requirements in the system python

Signed-off-by: Eduardo Apolinario <[email protected]>

* Rename setup-uv to setup-global-uv

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix call to install flytekit in dev image

Signed-off-by: Eduardo Apolinario <[email protected]>

* Comment out the dolt unit tests

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix papermill and great expectations enviroment setup

Signed-off-by: Eduardo Apolinario <[email protected]>

---------

Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Jan Fiedler <[email protected]>
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.

2 participants