-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat: generate api paths from sentry.api.urls #97155
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
Conversation
…own api url types
the params are checked inside `getApiUrl`, so it's fine to pass `path:undefined` here
|
🚨 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #97155 +/- ##
==========================================
- Coverage 81.36% 79.12% -2.25%
==========================================
Files 8535 8643 +108
Lines 378006 393114 +15108
Branches 23839 24577 +738
==========================================
+ Hits 307571 311042 +3471
- Misses 70068 81706 +11638
+ Partials 367 366 -1 |
# Conflicts: # static/app/api/apiOptions.ts
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
superseded by: |
This script is wired into pre-commit, so that whenever a `urls.py` file changes in `sentry` we re-build the list in typescript. For `getsentry`, because it's a separate repo, we can't have the same automation to keep the files in sync :(. So that list is broken out into it's own file and will need to manually be managed for now. ### Pre commit I triggered the pre-commit step on any urls.py file becaues we have our main `src/sentry/api/urls.py` file, but also a few places that contribute imports like `src/sentry/workflow_engine/endpoints/urls.py` and `src/sentry/notifications/platform/api/endpoints/urls.py`. I'd rather not have these extra urls.py file at all, and have all api routes be defined in one py file. But even if i go fix it now, someone is bound to add an import to import their route definitions again in the future. Ok, so we have a pre-commit. It will generate/update the ts file. ### CI tasks Changing the TS file does not trigger a full eslint/jest run in the frontend CI. Also it does not add the "Scope: Frontend" label to the PR. Only the TypeScript command is executed when the generated file changes. This will catch cases where someone trys to remove an api that is still in use (assuming all api call sites are statically typed, which they are not yet). Adding a new api endpoint in python will trigger TS, but shouldn't ever fail. If someone manages to add a new endpoint and the ts file doesn't re-generate then it won't be the end of the world. In the short term we'll just re-gen the file. ### Testing the Script There's about ~800 routes that the script identified. I got an llm to generate all the regexp that splits url patterns into N strings. It appears to be handling cases where a url segment is named, like `/(?P<issue_id>[^/]+)/` or has alternates like `/(issue|group)/`. With these examples working i didn't look through all 800 urls to validate them, I also didn't look for anything that's missing. As we statically type all our frontend code using this list, if something is wrong we can revisit the script and fix it up. There are however tests (llm generated) that are passing. ### Running the script The generated file has the command embedded inside it, and we can also see from the pre-commit-config: `python3 -m tools.api_urls_to_typescript` will do it. It can also be run with a flag: - `python3 -m tools.api_urls_to_typescript list` -> output to the terminal a list of urls, no formatting Related to #97155 <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Add Python tool to generate TS typings for API URLs from Django URL patterns, commit the generated file, add tests, and wire into pre-commit and CI filters. > > - **Tools / Codegen**: > - Add `tools/api_urls_to_typescript.py` to derive routes from Django `urlpatterns` and emit `static/app/api/urlpatterns.generated.ts` (with `KnownApiUrls` and `MaybeApiPath`). > - Supports listing routes via `python3 -m tools.api_urls_to_typescript list`. > - **Generated Artifacts**: > - Add `static/app/api/urlpatterns.generated.ts` containing the enumerated API paths. > - **Tests**: > - Add `tests/tools/test_api_urls_to_typescript.py` covering `regexp_to_routes` (named groups, alternates, optional parts, complex patterns). > - **Pre-commit**: > - Add `gen-ts-api-urls` hook to regenerate TS routes on `urls.py` changes. > - **CI / Filters**: > - Update `.github/file-filters.yml` to broaden TS/JS globs and exclude `*.generated.ts`; add frontend workflow anchor and include in triggers. > - Update `frontend.yml` job outputs to expose `typecheckable_rules_changed`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 82c5d6a. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
No description provided.