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

ci: setup build/publish with uv #573

Merged
merged 6 commits into from
Oct 31, 2024
Merged

ci: setup build/publish with uv #573

merged 6 commits into from
Oct 31, 2024

Conversation

domoritz
Copy link
Member

No description provided.

@domoritz domoritz requested a review from manzt October 30, 2024 21:51
Co-authored-by: Trevor Manz <trevor.j.manz@gmail.com>
"publish": "hatch publish --user __token__",
"release": "npm run prepublishOnly && npm run publish"
"prepublishOnly": "uv run pytest && uv run ruff check && uv run ruff format --check",
"publish": "uv build && uvx twine upload --skip-existing dist/*",
Copy link
Collaborator

@manzt manzt Oct 30, 2024

Choose a reason for hiding this comment

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

Don't mind this at all! We do something similar in anywidget. The funny thing is that uv is so flexible that you could probably publish a package to PyPI that just reads scripts from somewhere in the pyproject.toml and calls it (e.g., a python package called project-scripts that fulfills the following):

uvx project-scripts <script-name>

Copy link
Member Author

Choose a reason for hiding this comment

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

Haha, that's a nice hack. I hope uv will support scripts/shortcuts/commands/dev-scripts (whatever they will call it) eventually to clean this up.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed. IIRC, there has been some discussion around this, but I don't remember the thread or where I saw it.

@domoritz
Copy link
Member Author

The only thing left here is maybe using a workspace. Thoughts?

@domoritz domoritz requested a review from manzt October 31, 2024 02:43
@domoritz
Copy link
Member Author

I switched to a workspace for the top-level so we can use the same dependencies and simplify re-using packages across the workspace. @manzt can you take a final look before merging?

@manzt
Copy link
Collaborator

manzt commented Oct 31, 2024

ha - I was just checking our the branch to poke around. This definitely seems like the right setup for a workspace (the shared lock file is ideal). I'm thinking you could also probably move some of the ruff config eventually here and run it from the root of the repo.

One thing I wasn't sure about with workspaces is the support for an "anonymous" root project, that is a pyprojec.toml without any project metadata. All the uv workspace docs only seem to discuss a hybrid of packages + root ... but I just checked out the branch and it's really nice!

@manzt
Copy link
Collaborator

manzt commented Oct 31, 2024

BTW, you can build packages from the root simply with:

uv build --package duckdb-server
uv build --package mosaic-widget

and it will put the outputs into the root ./dist.

Copy link
Collaborator

@manzt manzt left a comment

Choose a reason for hiding this comment

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

LGTM! The workspaces is really nice. Just a small comment about CI and moving around ruff stuff but I think up for debate/ for a separate PR

Comment on lines 54 to 66
uv build
uv run ruff check
uv run ruff format --check

- name: Build, lint, and test Server
run: |
cd packages/duckdb-server
hatch build
hatch fmt --check
hatch run test:cov

uv build
uv run ruff check
uv run ruff format --check
uv run mypy
uv run --with pytest-cov pytest --cov-report=term-missing --color=yes --cov=pkg
Copy link
Collaborator

Choose a reason for hiding this comment

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

Depending on how you want to split out these, I believe you can run:

uv run ruff check
uv run ruff format --check

from the root of the repo.

I usally have a separate workflow for "Python Lint" e.g.,:

 python_lint:
    name: Lint Python
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4

      - uses: astral-sh/setup-uv@v3
        with:
          python-version-file: ".python-version"
          cache: "pip"
          cache-dependency-path: "**/pyproject.toml"

      - name: Ruff
        run: |
          uv run ruff check
          uv run ruff format --check
          
  python_test:
    name: Test Python
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4

      - uses: astral-sh/setup-uv@v3
        with:
          python-version-file: ".python-version"
          cache: "pip"
          cache-dependency-path: "**/pyproject.toml"

      - name: Widget
        run: |
          cd packages/widget
          uv build

      - name: Server
        run: |
          cd packages/duckdb-server
          uv run mypy
          uv run --with pytest-cov pytest --cov-report=term-missing --color=yes --cov=pkg

Copy link
Member Author

Choose a reason for hiding this comment

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

I just moved out up. I didn't set up a separate workflow since the python one is plenty fast.

setup-uv doesn't seem to like the python version file.

Screenshot 2024-10-31 at 10 43 57

Copy link
Collaborator

@manzt manzt Oct 31, 2024

Choose a reason for hiding this comment

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

setup-uv doesn't seem to like the python version file.

Yeah, uv is a python installer and by default i think picks a "managed" mode where it will determine and install interpreters rather than relying on the system. If there is a .python-version root of the project (like we've done in this PR) it will respect that when running various uv commands. You can specify the version with UV_PYTHON= in the env if you want to test multple python versions in matrix.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you could setup-python with a specific version and then setup-uv and run uv commands with --system to use the "system python" (i.e., whatever setup-python did), but i find the uv-only workflow most simple.

Copy link
Collaborator

Choose a reason for hiding this comment

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

https://github.com/manzt/anywidget/blob/478e742eaaf85d8fab68b006210680232863a488/.github/workflows/ci.yml#L56-L69

For example, I should change this to:

  TestPython:
    name: Python / Test
    runs-on: ubuntu-latest
    strategy:
      matrix:
        python-version:
          - "3.8"
          - "3.9"
          - "3.10"
          - "3.11"
          - "3.12"
    steps:
      - uses: actions/checkout@v4
      - uses: pnpm/action-setup@v4
        with:
          run_install: true
      - uses: astral-sh/setup-uv@v3
        with:
          version: "0.4.x"
      - name: Run tests
        run: uv run --with pytest-cov pytest ./tests --color=yes --cov anywidget --cov-report xml
        env:
          UV_PYTHON: ${{ matrix.python-version }}

@domoritz
Copy link
Member Author

One thing I wasn't sure about with workspaces is the support for an "anonymous" root project, that is a pyprojec.toml without any project metadata. All the uv workspace docs only seem to discuss a hybrid of packages + root ... but I just checked out the branch and it's really nice!

Yeah, I feel most of the time I don't want a root project and I'm glad uv is happily doing this. The docs could probably be updated to describe the anonymous workspace approach.

I'm thinking you could also probably move some of the ruff config eventually here and run it from the root of the repo.

Done in 9314395

@manzt
Copy link
Collaborator

manzt commented Oct 31, 2024

Yeah, I feel most of the time I don't want a root project and I'm glad uv is happily doing this.

Agreed, nice to know it works!

@domoritz domoritz merged commit c711428 into main Oct 31, 2024
3 checks passed
@domoritz domoritz deleted the dom/uv-2 branch October 31, 2024 15:01
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