Skip to content
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

Fixes #4703 Implement API for obtaining prohibited names #8533

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

zooba
Copy link
Contributor

@zooba zooba commented Sep 9, 2020

This is very much a work in progress for implementing #4703, but I wanted to get feedback on the direction as I go.

In particular, the API route can't (trivially) go under /api/prohibited_project_name because that's a lookup for a package by that name. I replaced the slash with an underscore so I could test it, but open to other ideas on where to put it.

At least in my dev environment, putting it at /admin/prohibited_project_name/json seems to work okay. But totally open to other suggestions.

@zooba
Copy link
Contributor Author

zooba commented Sep 9, 2020

Ping @di

@zooba
Copy link
Contributor Author

zooba commented Sep 9, 2020

Going to move it under the admin section where the existing route is, but leave it unauthenticated and just return the list as JSON.

@ewdurbin
Copy link
Member

ewdurbin commented Sep 9, 2020

we really shouldn't place this under /admin as that is explicitly not cached.

@ewdurbin
Copy link
Member

ewdurbin commented Sep 9, 2020

Speaking of caching, we'll want to instrument some way of purging this URL when a new project is prohibited or removed from the prohibited list.

@zooba
Copy link
Contributor Author

zooba commented Sep 9, 2020

If we move it under a route that Fastly will cache, will they respect expiry headers? This is an information API, so there's really no harm in it being up to a day behind.

@zooba
Copy link
Contributor Author

zooba commented Sep 9, 2020

Also, any suggestions/preferences for where to route it? Under /api/{project_name} can't work (unless we go ahead and ban the package name matching the API... 👀) None of the other existing paths looked like good options.

@zooba zooba marked this pull request as ready for review September 14, 2020 23:52
Base automatically changed from master to main January 21, 2021 18:39
@zooba zooba requested a review from a team as a code owner February 22, 2024 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants