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

Refactor metadata to blueprint/service #2039

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

mikesmit
Copy link
Collaborator

@mikesmit mikesmit commented Dec 4, 2024

Fixes #1992

This change migrates the existing //metadata route to a blueprint. This involved

  1. Creating a blueprint for just metadata which adds the //metadata route
  2. Updating the get_metadata function to use a MetadataService instead of directly referencing COUNTRIES.

Existing build tests already cover this functionality, but I also ran debug and verified with curl that the route responded appropriately.

Fixes #1992

This change migrates the existing /<country>/metadata route to a
blueprint. This involved
1. Creating a blueprint for just metadata which adds the
   /<country>/metadata route
2. Updating the get_metadata function to use a MetadataService instead
   of directly referencing COUNTRIES.

Existing build tests already cover this functionality, but I also ran
debug and verified with curl that the route responded appropriately.
@mikesmit mikesmit assigned anth-volk and unassigned anth-volk Dec 4, 2024
@mikesmit mikesmit requested a review from anth-volk December 4, 2024 15:46
Copy link
Collaborator

@anth-volk anth-volk left a comment

Choose a reason for hiding this comment

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

Thanks for your contributions to this @mikesmit! I had two requested changes related to a point you had brought up within the issue itself around what constitutes an "entity" within the API. If you'd prefer to discuss this and collectively make adjustments moving forward to how we do this, I'm totally open to it. Also open to pushing that to a second refactor at a later point. Looking forward to hearing your thoughts.

policyengine_api/api.py Show resolved Hide resolved
policyengine_api/routes/metadata_routes.py Show resolved Hide resolved
@anth-volk anth-volk self-requested a review December 9, 2024 19:24
Copy link
Collaborator

@anth-volk anth-volk left a comment

Choose a reason for hiding this comment

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

Thanks again for your work on this @mikesmit. I've re-reviewed, will be approving and merging, then revisiting my own related PRs based on our discussion.

@anth-volk anth-volk merged commit 670e6d4 into master Dec 9, 2024
4 checks passed
@anth-volk anth-volk deleted the 1992_metadata_endpoint_to_blueprint branch December 9, 2024 19:29
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.

Migrate metadata endpoint to new API structure
2 participants