add GHA skeleton for routing regression pipeline #12
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,85 @@ | ||||||
| name: Build Tiles | ||||||
|
|
||||||
| on: | ||||||
| workflow_dispatch: | ||||||
| inputs: | ||||||
| valhalla_ref: | ||||||
| description: "Valhalla ref to build tiles with (e.g. master)" | ||||||
| default: master | ||||||
|
|
||||||
| jobs: | ||||||
| build-valhalla: | ||||||
| uses: ./.github/workflows/build-valhalla.yml | ||||||
| with: | ||||||
| valhalla_ref: ${{ inputs.valhalla_ref }} | ||||||
| secrets: inherit | ||||||
|
Comment on lines
+12
to
+16
Collaborator
Author
There was a problem hiding this comment. build-valhalla now produces a wheel artifact that build-tiles downloads and installs, no rebuilding inline |
||||||
|
|
||||||
| build-tiles: | ||||||
| runs-on: ubuntu-24.04 | ||||||
|
Member
There was a problem hiding this comment.
Suggested change
Member
There was a problem hiding this comment. we want to know early if there's anything failing with current releases |
||||||
| needs: build-valhalla | ||||||
| steps: | ||||||
| - name: Checkout RAD | ||||||
| uses: actions/checkout@v4 | ||||||
| with: | ||||||
| lfs: true | ||||||
|
|
||||||
| - name: Checkout Valhalla | ||||||
| uses: actions/checkout@v4 | ||||||
| with: | ||||||
| repository: valhalla/valhalla | ||||||
| ref: ${{ inputs.valhalla_ref }} | ||||||
| submodules: recursive | ||||||
| path: valhalla-src | ||||||
|
|
||||||
| - name: Restore ccache | ||||||
| uses: tespkg/actions-cache/restore@v1 | ||||||
| with: | ||||||
| endpoint: ${{ secrets.HETZNER_S3_ENDPOINT }} | ||||||
| accessKey: ${{ secrets.HETZNER_S3_ACCESS_KEY }} | ||||||
| secretKey: ${{ secrets.HETZNER_S3_ACCESS_SECRET }} | ||||||
| bucket: rad-cache | ||||||
| path: ~/.cache/ccache | ||||||
| key: ccache-valhalla-${{ inputs.valhalla_ref }} | ||||||
| restore-keys: ccache-valhalla-master | ||||||
|
|
||||||
| - name: Install dependencies | ||||||
| run: bash valhalla-src/scripts/install-linux-deps.sh | ||||||
|
|
||||||
| - name: Rebuild Valhalla | ||||||
| run: | | ||||||
| cmake -B valhalla-src/build -S valhalla-src \ | ||||||
| -DCMAKE_BUILD_TYPE=Release \ | ||||||
| -DENABLE_PYTHON_BINDINGS=ON \ | ||||||
| -DENABLE_SERVICES=OFF \ | ||||||
| -DENABLE_TESTS=OFF \ | ||||||
| -DENABLE_BENCHMARKS=OFF \ | ||||||
| -DENABLE_CCACHE=ON | ||||||
| make -C valhalla-src/build -j$(nproc) | ||||||
| sudo make -C valhalla-src/build install | ||||||
|
|
||||||
| - name: Save ccache | ||||||
| if: always() | ||||||
| uses: tespkg/actions-cache/save@v1 | ||||||
| with: | ||||||
| endpoint: ${{ secrets.HETZNER_S3_ENDPOINT }} | ||||||
| accessKey: ${{ secrets.HETZNER_S3_ACCESS_KEY }} | ||||||
| secretKey: ${{ secrets.HETZNER_S3_ACCESS_SECRET }} | ||||||
| bucket: rad-cache | ||||||
| path: ~/.cache/ccache | ||||||
| key: ccache-valhalla-${{ inputs.valhalla_ref }} | ||||||
|
|
||||||
| - name: Build graph tiles | ||||||
| # TODO: admins.sqlite (500MB) not in repo — needs separate solution | ||||||
| # TODO: replace inline shell with scripts/build_tiles.py | ||||||
| run: | | ||||||
| valhalla_build_config \ | ||||||
| --mjolnir-tile-dir valhalla_tiles \ | ||||||
| --mjolnir-admin admins.sqlite > valhalla.json | ||||||
| valhalla_build_tiles -c valhalla.json data/liechtenstein_graph.osm.pbf | ||||||
|
|
||||||
| - name: Upload tiles artifact | ||||||
| uses: actions/upload-artifact@v4 | ||||||
| with: | ||||||
| name: valhalla-tiles-${{ inputs.valhalla_ref }} | ||||||
| path: valhalla_tiles/ | ||||||
| retention-days: 90 | ||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| name: Build Valhalla | ||
|
|
||
| on: | ||
| workflow_call: | ||
| inputs: | ||
| valhalla_ref: | ||
| description: "Valhalla git ref to build (branch, tag, or SHA)" | ||
| type: string | ||
| required: true | ||
|
|
||
| jobs: | ||
| build: | ||
| runs-on: ubuntu-24.04 | ||
| steps: | ||
| - name: Checkout Valhalla | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| repository: valhalla/valhalla | ||
| ref: ${{ inputs.valhalla_ref }} | ||
| submodules: recursive | ||
| path: valhalla-src | ||
|
|
||
| - name: Restore ccache | ||
| uses: tespkg/actions-cache/restore@v1 | ||
|
Member
There was a problem hiding this comment. actually: let's not use S3 for ccache after all. in the end we'll rarely build this, so we can use github's provided cache, it's 10 GB, more than comfortable for our project. |
||
| with: | ||
| endpoint: ${{ secrets.HETZNER_S3_ENDPOINT }} | ||
|
Member
There was a problem hiding this comment. where did you get this secret from? |
||
| accessKey: ${{ secrets.HETZNER_S3_ACCESS_KEY }} | ||
| secretKey: ${{ secrets.HETZNER_S3_ACCESS_SECRET }} | ||
| bucket: rad-cache | ||
| path: ~/.cache/ccache | ||
| key: ccache-valhalla-${{ inputs.valhalla_ref }} | ||
| restore-keys: ccache-valhalla-master | ||
|
|
||
| - name: Install dependencies | ||
| # TODO: replace with targeted dep install once GHA base image is confirmed | ||
| run: bash valhalla-src/scripts/install-linux-deps.sh | ||
|
Comment on lines
+47
to
+48
Member
There was a problem hiding this comment. first install, then restore cache |
||
|
|
||
| - name: Build Valhalla | ||
| run: | | ||
| cmake -B valhalla-src/build -S valhalla-src \ | ||
| -DCMAKE_BUILD_TYPE=Release \ | ||
| -DENABLE_PYTHON_BINDINGS=ON \ | ||
| -DENABLE_SERVICES=OFF \ | ||
| -DENABLE_TESTS=OFF \ | ||
| -DENABLE_BENCHMARKS=OFF \ | ||
| -DENABLE_CCACHE=ON | ||
| make -C valhalla-src/build -j$(nproc) | ||
| sudo make -C valhalla-src/build install | ||
|
|
||
| - name: Save ccache | ||
| if: always() | ||
| uses: tespkg/actions-cache/save@v1 | ||
| with: | ||
| endpoint: ${{ secrets.HETZNER_S3_ENDPOINT }} | ||
| accessKey: ${{ secrets.HETZNER_S3_ACCESS_KEY }} | ||
| secretKey: ${{ secrets.HETZNER_S3_ACCESS_SECRET }} | ||
| bucket: rad-cache | ||
| path: ~/.cache/ccache | ||
| key: ccache-valhalla-${{ inputs.valhalla_ref }} | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,146 @@ | ||
| name: Routing Regression | ||
|
|
||
| on: | ||
| workflow_dispatch: | ||
| inputs: | ||
| valhalla_ref_old: | ||
| description: "Valhalla ref for old router (e.g. master)" | ||
| default: master | ||
| valhalla_ref_new: | ||
| description: "Valhalla ref for new router (e.g. PR branch or SHA)" | ||
| required: true | ||
|
Comment on lines
+13
to
+15
Member
There was a problem hiding this comment. this is pretty brittle and awkward to hunt down. |
||
| tiles_artifact: | ||
| description: "Tiles artifact name to use (e.g. valhalla-tiles-master)" | ||
| default: valhalla-tiles-master | ||
|
|
||
| jobs: | ||
| build-valhalla-old: | ||
| uses: ./.github/workflows/build-valhalla.yml | ||
| with: | ||
| valhalla_ref: ${{ inputs.valhalla_ref_old }} | ||
| secrets: inherit | ||
|
|
||
| build-valhalla-new: | ||
| uses: ./.github/workflows/build-valhalla.yml | ||
| with: | ||
| valhalla_ref: ${{ inputs.valhalla_ref_new }} | ||
| secrets: inherit | ||
|
|
||
|
Comment on lines
+18
to
+29
Member
There was a problem hiding this comment. here we're building both SHAs again. once for build-tiles.yml, now for route regression test. we only need to build valhalla (bindings) once, if we do it right. that's a good 20 mins of CI time saved
Member
There was a problem hiding this comment. I really wonder why you build valhalla here again? I mean I guess the whole point of you pushing the wheel artifact from |
||
| run-routes: | ||
| runs-on: ubuntu-24.04 | ||
| needs: [build-valhalla-old, build-valhalla-new] | ||
| strategy: | ||
| matrix: | ||
| router: | ||
| - ref: ${{ inputs.valhalla_ref_old }} | ||
| name: old | ||
| - ref: ${{ inputs.valhalla_ref_new }} | ||
| name: new | ||
| steps: | ||
| - name: Checkout RAD | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Checkout RAD-data | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| repository: valhalla/RAD-data | ||
| path: RAD-data | ||
| token: ${{ secrets.RAD_DATA_TOKEN }} | ||
|
nilsnolde marked this conversation as resolved.
Outdated
Member
There was a problem hiding this comment. again, it's the same token. it's the 2nd time I'm reviewing this line and asking you to change it. please make sure already fixed things stay fixed! |
||
|
|
||
| - name: Checkout Valhalla at router ref | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| repository: valhalla/valhalla | ||
| ref: ${{ matrix.router.ref }} | ||
| submodules: recursive | ||
| path: valhalla-src | ||
|
|
||
| - name: Download tiles artifact | ||
| # TODO: when tiles come from a previous run, actions/download-artifact@v4 | ||
| # only fetches from the current run — needs cross-run solution via GitHub API | ||
| uses: actions/download-artifact@v4 | ||
| with: | ||
| name: ${{ inputs.tiles_artifact }} | ||
|
Member
There was a problem hiding this comment. per your description of the input: I'm not quite sure how to handle this exactly.. artifacts work per workflow run, meaning you'd always need to build both "old" and "new" graphs in each run. if we use artifacts for this. let me think for a second.
Member
There was a problem hiding this comment. this still stands @Sherley-Sonali, I still don't see how this could make sense with artifacts. nothing here actually says "build tiles". there's apparently some GHA config for it, but who calls that? |
||
| path: valhalla_tiles/ | ||
|
|
||
| - name: Restore ccache | ||
| uses: tespkg/actions-cache/restore@v1 | ||
| with: | ||
| endpoint: ${{ secrets.HETZNER_S3_ENDPOINT }} | ||
| accessKey: ${{ secrets.HETZNER_S3_ACCESS_KEY }} | ||
| secretKey: ${{ secrets.HETZNER_S3_ACCESS_SECRET }} | ||
| bucket: rad-cache | ||
| path: ~/.cache/ccache | ||
| key: ccache-valhalla-${{ matrix.router.ref }} | ||
| restore-keys: ccache-valhalla-master | ||
|
|
||
| - name: Install dependencies | ||
| run: bash valhalla-src/scripts/install-linux-deps.sh | ||
|
|
||
| - name: Build Valhalla at router ref | ||
| run: | | ||
| cmake -B valhalla-src/build -S valhalla-src \ | ||
| -DCMAKE_BUILD_TYPE=Release \ | ||
| -DENABLE_PYTHON_BINDINGS=ON \ | ||
| -DENABLE_SERVICES=OFF \ | ||
| -DENABLE_TESTS=OFF \ | ||
| -DENABLE_BENCHMARKS=OFF \ | ||
| -DENABLE_CCACHE=ON | ||
| make -C valhalla-src/build -j$(nproc) | ||
| sudo make -C valhalla-src/build install | ||
|
|
||
| - name: Save ccache | ||
| if: always() | ||
| uses: tespkg/actions-cache/save@v1 | ||
| with: | ||
| endpoint: ${{ secrets.HETZNER_S3_ENDPOINT }} | ||
| accessKey: ${{ secrets.HETZNER_S3_ACCESS_KEY }} | ||
| secretKey: ${{ secrets.HETZNER_S3_ACCESS_SECRET }} | ||
| bucket: rad-cache | ||
| path: ~/.cache/ccache | ||
| key: ccache-valhalla-${{ matrix.router.ref }} | ||
|
|
||
| - name: Run route requests | ||
| # TODO: implement scripts/run_routes.py | ||
| # reads RAD-data/requests/requests.jsonl | ||
| # uses pyvalhalla bindings built from source | ||
| # writes to RAD-data/responses/${{ matrix.router.name }}.jsonl | ||
| run: | | ||
| echo "TODO: python3 scripts/run_routes.py \ | ||
| --requests RAD-data/requests/requests.jsonl \ | ||
| --tiles valhalla_tiles/ \ | ||
| --output RAD-data/responses/${{ matrix.router.name }}.jsonl" | ||
|
|
||
| - name: Push responses to RAD-data | ||
| # TODO: implement scripts/push_results.py | ||
| # race condition risk if both matrix jobs push simultaneously — | ||
| # consider pushing responses as GHA artifacts instead, let diff-and-store handle RAD-data push | ||
| run: | | ||
| echo "TODO: python3 scripts/push_results.py --repo RAD-data --stage responses" | ||
|
|
||
| diff-and-store: | ||
| runs-on: ubuntu-24.04 | ||
| needs: run-routes | ||
| if: always() | ||
| steps: | ||
| - name: Checkout RAD-data | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| repository: valhalla/RAD-data | ||
| path: RAD-data | ||
| token: ${{ secrets.RAD_DATA_TOKEN }} | ||
|
|
||
| - name: Compute diff | ||
| # TODO: implement scripts/diff_responses.py | ||
| # reads RAD-data/responses/old.jsonl and new.jsonl | ||
| # writes RAD-data/diffs/<run-id>.json | ||
| run: | | ||
| echo "TODO: python3 scripts/diff_responses.py \ | ||
| --old RAD-data/responses/old.jsonl \ | ||
| --new RAD-data/responses/new.jsonl \ | ||
| --output RAD-data/diffs/${{ github.run_id }}.json" | ||
|
|
||
| - name: Push diff to RAD-data | ||
| # TODO: implement scripts/push_results.py | ||
| run: | | ||
| echo "TODO: python3 scripts/push_results.py --repo RAD-data --stage diff" | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ha, you even use it here. but again, it's a redundant job, see #12 (comment). you do that in
build-tilesbelow without any artifacts.