ci: Fix CI step that generates Typescript enums based on api urls #101604
Conversation
github-actions
bot
added
Scope: Frontend
|
🚨 Warning: This pull request contains Frontend and Backend changes! It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently. Have questions? Please ask in the |
ryan953
force-pushed
the
ryan953/test-ci-linting
branch
from
6e53a84 to
7205597
Compare
ryan953
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| - 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() |
There was a problem hiding this comment.
Bug: Token Generation Fails Due to Undefined Variable
The api-url-typescript job's token generation and commit steps use env.SECRET_ACCESS == 'true'. Since SECRET_ACCESS is undefined, these steps are always skipped, preventing the job from obtaining a token or committing generated TypeScript changes. This differs from the pattern in other similar jobs.
| backend_api_urls: &backend_api_urls | ||
| - '**/urls.py' |
There was a problem hiding this comment.
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.
it kinda mis-matches on purpose, there are gaps too.
The main entry file is src/sentry/api/urls.py but that also imports from:
src/sentry/workflow_engine/endpoints/urls.pysrc/sentry/notifications/platform/api/endpoints/urls.pysrc/sentry/preprod/api/endpoints/urls.py- and maybe more in the future.
So 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() imo
There was a problem hiding this comment.
we can add a comment maybe too
Followup to a comment in #101604
This throws the command into it's own CI step, so that it can do all the setup stuff only if a urls.py file is changed, and it will still auto-update the PR if changes are needed.
This keeps CI fast for the bulk of PRs where we don't need to run setup, and don't need to do the check.
With pre-commit i could only figure out how to do the setup step unconditionally before pre-commit ran, it would've been some extra coupling to make that conditional and it would be non-obvious which pre-commit step needs it. So no more pre-commit for now.