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.

Sign up for GitHub

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

Jump to bottom

Make endpoints work with or without a trailing slash #886

Merged
merged 5 commits into from
Jul 12, 2022
Merged

Make endpoints work with or without a trailing slash #886

merged 5 commits into from
Jul 12, 2022

Conversation

allisonking
Copy link
Contributor

@allisonking allisonking commented Jul 8, 2022 •
edited by ThomasLaPiana
Loading

Closes #690

Code Changes

  • Adds a custom APIRouter that allows routes both with and without trailing slashes
  • Imports this router instead of the default FastAPI one
  • Refactors the health and admin/db endpoints to their own files. In order for them to also get the trailing slash benefit, they had to also use the custom APIRouter, and not the default @app from main.py
  • Tests

Steps to Confirm

  • nox -s dev and then try endpoints with and without a slash! 🎉

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Documentation Updated:
    • documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
    • documentation issue created (tag docs-team to complete issue separately)
  • Issue Requirements are Met
  • Update CHANGELOG.md

Description Of Changes

Mostly copy pasta from @TheAndrewJackson 's good work :) ethyca/fidesops@a072f2a

And then some refactoring to pull some endpoints into their own files c41c7b6

@allisonking allisonking changed the title Use custom APIRouter Make endpoints work with or without a trailing slash Jul 8, 2022
@allisonking allisonking marked this pull request as ready for review July 9, 2022 02:33
@allisonking allisonking requested a review from a team July 9, 2022 02:33
ThomasLaPiana
ThomasLaPiana approved these changes Jul 11, 2022
View reviewed changes
Copy link
Contributor

@ThomasLaPiana ThomasLaPiana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love to see it! Clean solution (shout out @TheAndrewJackson ) and the additional cleanup was a nice touch too

@ThomasLaPiana
Copy link
Contributor

ok I ruined this a bit when I merged another PR, I'll fix the conflict

@allisonking allisonking merged commit 4065494 into main Jul 12, 2022
@allisonking allisonking deleted the aking-690-trailing-slash branch July 12, 2022 14:09
allisonking added a commit that referenced this pull request Jul 28, 2022
@allisonking @ThomasLaPiana
Make endpoints work with or without a trailing slash (#886)
d973ef1 
* Use custom APIRouter

* Add tests

* Refactor main.py to separate out Health and Admin endpoints

* Fix patch

Co-authored-by: Thomas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ThomasLaPiana ThomasLaPiana ThomasLaPiana approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Some endpoints only work with a trailing slash
2 participants
@allisonking @ThomasLaPiana