jiayuasu commented on code in PR #1134: URL: https://github.com/apache/sedona-db/pull/1134#discussion_r3739141471
########## .github/workflows/geopandas.yml: ########## @@ -0,0 +1,78 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +name: geopandas + +# Fast feedback for sedonadb-geopandas: install the published nightly sedonadb +# and sedonadb-expr wheels instead of building them from source, so a change to +# this package gets a result in about a minute rather than waiting on the wheel +# chain (which also needs every platform job to finish first). +# +# This is deliberately in addition to the coupled check in python.yml, not a +# replacement: this workflow answers "does the package work against a published +# sedonadb", while the source-built one answers "did a change to sedonadb break +# the package". Both are worth having. +on: + pull_request: + branches: + - main + paths: + - 'python/sedonadb-geopandas/**' + - '.github/workflows/geopandas.yml' + push: + branches: + - main + workflow_dispatch: + +concurrency: + group: ${{ github.repository }}-${{ github.ref }}-${{ github.workflow }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + test: + name: "sedonadb-geopandas (nightly sedonadb)" + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + # No submodules needed: this package is pure Python and nothing is built + # from source here. The checkout does need the root Cargo.toml, which the + # package reads to derive its version. + - uses: actions/checkout@v7 + + - uses: actions/setup-python@v7 + with: + python-version: '3.13' + + - name: Install sedonadb-geopandas against nightly sedonadb + # --pre together with the nightly index is what selects the nightly + # wheels: they version as X.Y.ZaN, which outranks the latest release, so + # the package's own floor (>=0.4.0) resolves to the nightly here. + run: | + pip install --pre \ + --extra-index-url https://pypi.fury.io/sedona-nightlies/ \ + "python/sedonadb-geopandas/[test]" Review Comment: That is a fair objection, and I have removed the workflow from this PR so the dependency fix can land without waiting on it. Happy to bring it back separately, reworked. You are right that it does not validate the branch: installing published nightly wheels means that on a PR touching both sedonadb and this package, the result says nothing about the combination — it could pass while a sedonadb change in the same PR breaks the package, or fail because the package uses something newer than the last nightly. For context on what I was reaching for, and where I got the framing wrong. It was not meant to be the authoritative check — `python.yml` already covers `python/**` and builds sedonadb from source, so for any change to this package the source-coupled run happens regardless. What I wanted was a *clean environment*, which is the part neither existing check gives cheaply: `python.yml` installs `sedonadb[test]`, and that extra happens to include `geoarrow-pyarrow`, so it structurally could not have caught the bug this PR fixes. The wheels job does run in a clean environment and did catch it, but only after every platform wheel builds, which is why it surfaced after merge rather than before. So the useful property was "clean environment", and pulling the dependency from nightly was just the cheap way to avoid a Rust build. Naming it as a build against nightly overstated what it verified. If it comes back, it should either be scoped so it cannot produce a misleading signal (skipped when a PR touches sedonadb or the Rust crates) or be framed for what it actually tests — that a released sedonadb satisfies this package, which is a real user scenario given the `sedonadb>=0.4.0` floor — rather than presented as a check of the branch. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
