-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
ci: Fix CI step that generates Typescript enums based on api urls #101604
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
Changes from all commits
9870fdb
0ce091f
7205597
9c2a014
d8e9b5b
40b5a69
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,7 @@ jobs: | |
| api_docs: ${{ steps.changes.outputs.api_docs }} | ||
| backend: ${{ steps.changes.outputs.backend_all }} | ||
| backend_dependencies: ${{ steps.changes.outputs.backend_dependencies }} | ||
| backend_api_urls: ${{ steps.changes.outputs.backend_api_urls }} | ||
| backend_any_type: ${{ steps.changes.outputs.backend_any_type }} | ||
| migration_lockfile: ${{ steps.changes.outputs.migration_lockfile }} | ||
| steps: | ||
|
|
@@ -245,6 +246,41 @@ jobs: | |
| github-token: ${{ steps.token.outputs.token }} | ||
| message: ':snowflake: re-freeze requirements' | ||
|
|
||
| api-url-typescript: | ||
| if: needs.files-changed.outputs.backend_api_urls == 'true' | ||
| needs: files-changed | ||
| name: api url typescript generation | ||
| runs-on: ubuntu-24.04 | ||
| timeout-minutes: 10 | ||
| steps: | ||
| - # get a non-default github token so that any changes are verified by CI | ||
| if: env.SECRET_ACCESS == 'true' | ||
| uses: getsentry/action-github-app-token@d4b5da6c5e37703f8c3b3e43abb5705b46e159cc # v3.0.0 | ||
| id: token | ||
| with: | ||
| app_id: ${{ vars.SENTRY_INTERNAL_APP_ID }} | ||
| private_key: ${{ secrets.SENTRY_INTERNAL_APP_PRIVATE_KEY }} | ||
|
|
||
| - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 | ||
|
|
||
| - name: Setup sentry env | ||
| uses: ./.github/actions/setup-sentry | ||
| id: setup | ||
| with: | ||
| mode: backend-ci | ||
|
|
||
| - name: Sync API Urls to TypeScirpt | ||
| run: | | ||
| python3 -m tools.api_urls_to_typescript | ||
|
|
||
| - name: Apply any file changes | ||
| # note: this runs "always" or else it's skipped when pre-commit fails | ||
| if: env.SECRET_ACCESS == 'true' && startsWith(github.ref, 'refs/pull') && always() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Token Generation Fails Due to Undefined VariableThe |
||
| uses: getsentry/action-github-commit@31f6706ca1a7b9ad6d22c1b07bf3a92eabb05632 # v2.0.0 | ||
| with: | ||
| github-token: ${{ steps.token.outputs.token }} | ||
| message: ':hammer_and_wrench: Sync API Urls to TypeScirpt' | ||
|
|
||
| migration: | ||
| if: needs.files-changed.outputs.migration_lockfile == 'true' | ||
| needs: files-changed | ||
|
|
||
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.
This might mis-match some things. Not huge problem but I wonder if we can be slightly more specific.
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.
it kinda mis-matches on purpose, there are gaps too.
The main entry file is
src/sentry/api/urls.pybut that also imports from:src/sentry/workflow_engine/endpoints/urls.pysrc/sentry/notifications/platform/api/endpoints/urls.pysrc/sentry/preprod/api/endpoints/urls.pySo i'm trying to catch them all for now. Like i said before, the script is sus, but good enough to get started adopting
getApiUrl()imoThere 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.
we can add a comment maybe too
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.
#101638