surrealdb / surrealdb.py

SurrealDB SDK for Python
https://surrealdb.com
Apache License 2.0
170 stars 49 forks source link

Restructure project to use `maturin` to build `pyo3` bindings #88

Closed Ce11an closed 3 months ago

Ce11an commented 3 months ago

Thank you for submitting this pull request! We appreciate you spending the time to work on these changes.

Motivation for the Change

Objective

Adopt maturin as the build system for surrealdb.py.

Background

The primary goal behind this change is to streamline the build process for Rust's pyo3 bindings within surrealdb.py. maturin stands out as a robust and reliable build system, a fact well-documented by its successful adoption in projects such as pydantic-core and ruff. Its integration into surrealdb.py is expected to significantly enhance the development workflow, alongside improving testing and release processes.

Type of Change

What does this change do?

Adds maturin as a build-system and removes now redundant files.

What is your testing strategy?

All tests are working. The "cross-build" jobs may need work. However, I cannot run those.

Is this related to any issues?

Have you read the Contributing Guidelines?

Ce11an commented 3 months ago

@maxwellflitton, what do you think about this change? I still need to adjust the CI jobs to install surrealdb.py with maturin. Linters, formatters, and static type-checkers should also be added. I can whack it all in one PR, or split it up. Thought I would get your opinion first. I probably should have made an issue... but it has nevertheless been fun to explore maturin.

Ce11an commented 3 months ago

@maxwellflitton, what do you think about this change? I still need to adjust the CI jobs to install surrealdb.py with maturin. Linters, formatters, and static type-checkers should also be added. I can whack it all in one PR, or split it up. Thought I would get your opinion first. I probably should have made an issue... but it has nevertheless been fun to explore maturin.

I fix the GitHub Actions. I have not changed the "cross-build" workflow. Some improvements could be made using maturin something like the below to build the distribution artefacts:

jobs:
  linux:
    runs-on: ubuntu-latest
    strategy:
      matrix:
        target: [x86_64, x86, aarch64, armv7, s390x, ppc64le]
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-python@v5
        with:
          python-version: '3.10'
      - name: Build wheels
        uses: PyO3/maturin-action@v1
        with:
          target: ${{ matrix.target }}
          args: --release --out dist --find-interpreter
          sccache: 'true'
          manylinux: auto
      - name: Upload wheels
        uses: actions/upload-artifact@v4
        with:
          name: wheels-linux-${{ matrix.target }}
          path: dist

  windows:
    runs-on: windows-latest
    strategy:
      matrix:
        target: [x64, x86]
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-python@v5
        with:
          python-version: '3.10'
          architecture: ${{ matrix.target }}
      - name: Build wheels
        uses: PyO3/maturin-action@v1
        with:
          target: ${{ matrix.target }}
          args: --release --out dist --find-interpreter
          sccache: 'true'
      - name: Upload wheels
        uses: actions/upload-artifact@v4
        with:
          name: wheels-windows-${{ matrix.target }}
          path: dist

  macos:
    runs-on: macos-latest
    strategy:
      matrix:
        target: [x86_64, aarch64]
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-python@v5
        with:
          python-version: '3.10'
      - name: Build wheels
        uses: PyO3/maturin-action@v1
        with:
          target: ${{ matrix.target }}
          args: --release --out dist --find-interpreter
          sccache: 'true'
      - name: Upload wheels
        uses: actions/upload-artifact@v4
        with:
          name: wheels-macos-${{ matrix.target }}
          path: dist

  sdist:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - name: Build sdist
        uses: PyO3/maturin-action@v1
        with:
          command: sdist
          args: --out dist
      - name: Upload sdist
        uses: actions/upload-artifact@v4
        with:
          name: wheels-sdist
          path: dist

  release:
    name: Release
    runs-on: ubuntu-latest
    if: "startsWith(github.ref, 'refs/tags/')"
    needs: [linux, windows, macos, sdist]
    steps:
      - uses: actions/download-artifact@v4
      - name: Publish to PyPI
        uses: PyO3/maturin-action@v1
        env:
          MATURIN_PYPI_TOKEN: ${{ secrets.PYPI_API_TOKEN }}
        with:
          command: upload
          args: --non-interactive --skip-existing wheels-*/*

I did want to add the ability to test the package on Windows and MacOS. However, there seems to be an issue between GitHub, requests, docker, MacOS, and Windows. I have managed to run the tests locally on my Mac so I am not sure what the issue is. Maybe one for another PR. See the below for more details:

I appreciate this is a big change, with a lot of clean up. If I got time, I will add in some standardisation with linters, formatters, and static type-checkers.

Let me know what you think!

maxwellflitton commented 3 months ago

@Ce11an thanks for this amazing work. it was on my to-do list and you've knocked it out of the park. Can you make the pull request to the test-deployment branch so I can start checking out the CI for deployment builds?

Ce11an commented 3 months ago

@Ce11an thanks for this amazing work. it was on my to-do list and you've knocked it out of the park. Can you make the pull request to the test-deployment branch so I can start checking out the CI for deployment builds?

Thanks, I had good fun doing it! I have updated the branch. Let me know if there is anything else I can assist with 👍🏻