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

Feature version aliasing #236

Merged
merged 18 commits into from
Oct 14, 2021
Merged

Conversation

solomon-negusse
Copy link
Member

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side). Don't request your master!
  • Make sure you are making a pull request against the develop branch (left side). Also you should start your branch off our develop.
  • Check the commit's or even all commits' message styles matches our requested structure.
  • Check your code additions will fail neither code linting checks nor unit test.

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Issue Number: 1405

What is the new behavior?

  • Part of data versioning feature. Adds version aliasing endpoints

Does this introduce a breaking change?

  • Yes
  • No

Other information

@codecov
Copy link

codecov bot commented Sep 28, 2021

Codecov Report

Merging #236 (5dee1a0) into feature/Revisions (f0d9aa8) will decrease coverage by 0.48%.
The diff coverage is 68.14%.

❗ Current head 5dee1a0 differs from pull request most recent head 8bef0fb. Consider uploading reports for the commit 8bef0fb to get more accurate results
Impacted file tree graph

@@                  Coverage Diff                  @@
##           feature/Revisions     #236      +/-   ##
=====================================================
- Coverage              85.87%   85.38%   -0.49%     
=====================================================
  Files                    113      118       +5     
  Lines                   4502     4626     +124     
=====================================================
+ Hits                    3866     3950      +84     
- Misses                   636      676      +40     
Flag Coverage Δ
unittests 85.38% <68.14%> (-0.49%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
app/routes/datasets/aliases.py 42.10% <42.10%> (ø)
app/crud/aliases.py 50.00% <50.00%> (ø)
app/middleware.py 81.42% <77.27%> (-1.91%) ⬇️
app/main.py 84.81% <100.00%> (+0.39%) ⬆️
app/models/orm/aliases.py 100.00% <100.00%> (ø)
...pp/models/orm/migrations/versions/a5787f2eefe5_.py 100.00% <100.00%> (ø)
app/models/pydantic/aliases.py 100.00% <100.00%> (ø)
app/routes/datasets/versions.py 88.02% <100.00%> (+0.08%) ⬆️
tests/routes/__init__.py 89.18% <0.00%> (-10.82%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f0d9aa8...8bef0fb. Read the comment docs.

@solomon-negusse solomon-negusse marked this pull request as draft September 30, 2021 14:01
@solomon-negusse solomon-negusse changed the base branch from develop to feature/Revisions October 13, 2021 16:22
- create middleware that redirects from version alias to version for supported ops
- create dependency that checks for alias conflict when creating a version
@solomon-negusse solomon-negusse marked this pull request as ready for review October 13, 2021 20:04
@@ -77,6 +77,37 @@ async def redirect_latest(request: Request, call_next):
return response


async def redirect_alias_to_version(request: Request, call_next):
Copy link
Member

Choose a reason for hiding this comment

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

Just so I understand correctly, this will check the request has /dataset/{dataset}/{version} in the path, and if so, check if it's an alias and redirect? Maybe add a doc string for this one to explain the behavior.

Copy link
Member Author

Choose a reason for hiding this comment

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

Exactly. I've updated docstring with the detail.

@solomon-negusse solomon-negusse merged commit 6f381a8 into feature/Revisions Oct 14, 2021
@solomon-negusse solomon-negusse deleted the feature-aliasing branch October 14, 2021 21:22
@jterry64 jterry64 restored the feature-aliasing branch December 18, 2024 20:20
@jterry64 jterry64 deleted the feature-aliasing branch December 18, 2024 20:20
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.

3 participants