Fix UV install in Makefile #2421

thomasjpfan · 2024-05-15T22:36:19Z

This PR fixes the CI issue in main.

https://github.com/flyteorg/flytekit/actions/runs/9100354302/job/25015010454

Signed-off-by: Thomas J. Fan <[email protected]>

eapolinario · 2024-05-15T22:46:34Z

why is this happening now? I see that CI checks in master are failing (the run you linked in the description).

Signed-off-by: Thomas J. Fan <[email protected]>

eapolinario · 2024-05-15T23:18:23Z

.github/workflows/pythonbuild.yml

      - name: Setup Flyte Sandbox
        run: |
-          flytectl demo start
+          flytectl demo start --version=v0.8.18


we don't need to specify the version here. This flag maps to a flyte release, not flytectl.

eapolinario · 2024-05-15T23:19:17Z

Makefile

@@ -30,7 +30,7 @@ setup: install-piptools ## Install requirements
 # Warning: this will install the requirements in your system python
 .PHONY: setup-global-uv
 setup-global-uv:
-	uv pip install --system -r dev-requirements.in
+	SETUPTOOLS_SCM_PRETEND_VERSION="3.0.0.dev0" uv pip install --system -r dev-requirements.in


I'm still confused. Why are we seeing this now and not when I introduced uv in #2403?

I'm curious about the reason too.

And after adding SETUPTOOLS_SCM_PRETEND_VERSION, it indeed fixed my broken CI.

feel like the regression in UV. they just released yesterday. https://pypi.org/project/uv/#history

This is unfortunate. I couldn't find any issue on their tracker though.

eapolinario · 2024-05-16T00:44:46Z

.github/workflows/pythonbuild.yml

@@ -259,9 +259,12 @@ jobs:
          uv pip freeze
      - name: Install FlyteCTL
        uses: unionai-oss/flytectl-setup-action@master
+        with:
+          # Remove version and go back to latest when unionai-oss/flytectl-setup-action is fixed


unionai-oss/flytectl-setup-action#5 is the fix.

You can revert this change. The above PR was merged and I tested it in flyteorg/flyte#5377.

austin362667 · 2024-05-16T03:47:16Z

Makefile

@@ -30,7 +30,7 @@ setup: install-piptools ## Install requirements
 # Warning: this will install the requirements in your system python
 .PHONY: setup-global-uv
 setup-global-uv:
-	uv pip install --system -r dev-requirements.in
+	SETUPTOOLS_SCM_PRETEND_VERSION="3.0.0.dev0" uv pip install --system -r dev-requirements.in


I'm curious about the reason too.

And after adding SETUPTOOLS_SCM_PRETEND_VERSION, it indeed fixed my broken CI.

Signed-off-by: Thomas J. Fan <[email protected]>

eapolinario · 2024-05-16T16:57:53Z

Makefile

@@ -30,7 +30,7 @@ setup: install-piptools ## Install requirements
 # Warning: this will install the requirements in your system python
 .PHONY: setup-global-uv
 setup-global-uv:
-	uv pip install --system -r dev-requirements.in
+	SETUPTOOLS_SCM_PRETEND_VERSION="3.0.0.dev0" uv pip install --system -r dev-requirements.in


This is unfortunate. I couldn't find any issue on their tracker though.

Signed-off-by: Thomas J. Fan <[email protected]>

codecov · 2024-05-16T19:36:24Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.41%. Comparing base (e6e08f9) to head (12cf5e8).
Report is 6 commits behind head on master.

❗ Current head 12cf5e8 differs from pull request most recent head c504943

Please upload reports for the commit c504943 to get more accurate results.

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #2421       +/-   ##
===========================================
+ Coverage   42.77%   90.41%   +47.63%     
===========================================
  Files         185       49      -136     
  Lines       18677     1910    -16767     
  Branches     2665        0     -2665     
===========================================
- Hits         7990     1727     -6263     
+ Misses      10599      183    -10416     
+ Partials       88        0       -88

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

Signed-off-by: Thomas J. Fan <[email protected]>

eapolinario

I'd still love to understand what change in uv caused this, but let's unblock for now.

* Fix UV install in Makefile Signed-off-by: Thomas J. Fan <[email protected]> * Fixes failing test Signed-off-by: Thomas J. Fan <[email protected]> * Use dev version to be simplier Signed-off-by: Thomas J. Fan <[email protected]> * Use v0.8.18 flytectl Signed-off-by: Thomas J. Fan <[email protected]> * Adds comment Signed-off-by: Thomas J. Fan <[email protected]> * Remove version specification Signed-off-by: Thomas J. Fan <[email protected]> * Remove version pin Signed-off-by: Thomas J. Fan <[email protected]> * Use pseudo version for integreation tests Signed-off-by: Thomas J. Fan <[email protected]> * Use a version that packaging recognizes Signed-off-by: Thomas J. Fan <[email protected]> * Remove quotes Signed-off-by: Thomas J. Fan <[email protected]> * Set flyteidl version Signed-off-by: Thomas J. Fan <[email protected]> * FIX Fixes version more for uv strictness Signed-off-by: Thomas J. Fan <[email protected]> --------- Signed-off-by: Thomas J. Fan <[email protected]>

eapolinario · 2024-05-24T01:28:08Z

I finally got to the bottom of this. It started to happen when I moved flytectl to the monorepo. Here's the fix: flyteorg/flyte#5419.

* Fix UV install in Makefile Signed-off-by: Thomas J. Fan <[email protected]> * Fixes failing test Signed-off-by: Thomas J. Fan <[email protected]> * Use dev version to be simplier Signed-off-by: Thomas J. Fan <[email protected]> * Use v0.8.18 flytectl Signed-off-by: Thomas J. Fan <[email protected]> * Adds comment Signed-off-by: Thomas J. Fan <[email protected]> * Remove version specification Signed-off-by: Thomas J. Fan <[email protected]> * Remove version pin Signed-off-by: Thomas J. Fan <[email protected]> * Use pseudo version for integreation tests Signed-off-by: Thomas J. Fan <[email protected]> * Use a version that packaging recognizes Signed-off-by: Thomas J. Fan <[email protected]> * Remove quotes Signed-off-by: Thomas J. Fan <[email protected]> * Set flyteidl version Signed-off-by: Thomas J. Fan <[email protected]> * FIX Fixes version more for uv strictness Signed-off-by: Thomas J. Fan <[email protected]> --------- Signed-off-by: Thomas J. Fan <[email protected]>

* Fix UV install in Makefile Signed-off-by: Thomas J. Fan <[email protected]> * Fixes failing test Signed-off-by: Thomas J. Fan <[email protected]> * Use dev version to be simplier Signed-off-by: Thomas J. Fan <[email protected]> * Use v0.8.18 flytectl Signed-off-by: Thomas J. Fan <[email protected]> * Adds comment Signed-off-by: Thomas J. Fan <[email protected]> * Remove version specification Signed-off-by: Thomas J. Fan <[email protected]> * Remove version pin Signed-off-by: Thomas J. Fan <[email protected]> * Use pseudo version for integreation tests Signed-off-by: Thomas J. Fan <[email protected]> * Use a version that packaging recognizes Signed-off-by: Thomas J. Fan <[email protected]> * Remove quotes Signed-off-by: Thomas J. Fan <[email protected]> * Set flyteidl version Signed-off-by: Thomas J. Fan <[email protected]> * FIX Fixes version more for uv strictness Signed-off-by: Thomas J. Fan <[email protected]> --------- Signed-off-by: Thomas J. Fan <[email protected]> Signed-off-by: Jan Fiedler <[email protected]>

Fix UV install in Makefile

7912dbb

Signed-off-by: Thomas J. Fan <[email protected]>

thomasjpfan marked this pull request as ready for review May 15, 2024 22:43

thomasjpfan requested review from wild-endeavor, kumare3, eapolinario, pingsutw, cosmicBboy and samhita-alla as code owners May 15, 2024 22:43

thomasjpfan added 4 commits May 15, 2024 18:47

Fixes failing test

475798f

Signed-off-by: Thomas J. Fan <[email protected]>

Use dev version to be simplier

926e588

Signed-off-by: Thomas J. Fan <[email protected]>

Use v0.8.18 flytectl

84a2e51

Signed-off-by: Thomas J. Fan <[email protected]>

Adds comment

dd8efe6

Signed-off-by: Thomas J. Fan <[email protected]>

eapolinario reviewed May 15, 2024

View reviewed changes

eapolinario reviewed May 16, 2024

View reviewed changes

austin362667 approved these changes May 16, 2024

View reviewed changes

Remove version specification

bd5cd24

Signed-off-by: Thomas J. Fan <[email protected]>

austin362667 approved these changes May 16, 2024

View reviewed changes

Remove version pin

a426ba0

Signed-off-by: Thomas J. Fan <[email protected]>

eapolinario previously approved these changes May 16, 2024

View reviewed changes

Use pseudo version for integreation tests

c91c191

Signed-off-by: Thomas J. Fan <[email protected]>

thomasjpfan dismissed eapolinario’s stale review via c91c191 May 16, 2024 18:52

eapolinario previously approved these changes May 16, 2024

View reviewed changes

Use a version that packaging recognizes

33de9c3

Signed-off-by: Thomas J. Fan <[email protected]>

thomasjpfan dismissed eapolinario’s stale review via 33de9c3 May 16, 2024 19:20

Remove quotes

12cf5e8

Signed-off-by: Thomas J. Fan <[email protected]>

thomasjpfan added 2 commits May 16, 2024 16:01

Set flyteidl version

f6a13e0

Signed-off-by: Thomas J. Fan <[email protected]>

FIX Fixes version more for uv strictness

c504943

Signed-off-by: Thomas J. Fan <[email protected]>

eapolinario approved these changes May 16, 2024

View reviewed changes

eapolinario merged commit edab1e3 into flyteorg:master May 16, 2024
45 of 46 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix UV install in Makefile #2421

Fix UV install in Makefile #2421

thomasjpfan commented May 15, 2024

eapolinario commented May 15, 2024

eapolinario May 15, 2024

eapolinario May 16, 2024

eapolinario May 15, 2024 •

edited

Loading

austin362667 May 16, 2024

pingsutw May 16, 2024

eapolinario May 16, 2024

eapolinario May 16, 2024

eapolinario May 16, 2024

austin362667 May 16, 2024

eapolinario May 16, 2024

codecov bot commented May 16, 2024 •

edited

Loading

eapolinario left a comment •

edited

Loading

eapolinario commented May 24, 2024

Fix UV install in Makefile #2421

Fix UV install in Makefile #2421

Conversation

thomasjpfan commented May 15, 2024

eapolinario commented May 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eapolinario May 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented May 16, 2024 • edited Loading

Codecov Report

eapolinario left a comment • edited Loading

Choose a reason for hiding this comment

eapolinario commented May 24, 2024

eapolinario May 15, 2024 •

edited

Loading

codecov bot commented May 16, 2024 •

edited

Loading

eapolinario left a comment •

edited

Loading