DPL-058-01: Versioned API #392
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #392 +/- ##
===========================================
+ Coverage 88.24% 88.26% +0.02%
===========================================
Files 38 39 +1
Lines 2723 2728 +5
Branches 318 318
===========================================
+ Hits 2403 2408 +5
Misses 280 280
Partials 40 40
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Nice and clean! I understand the reasoning behind starting in crawler but we were more concerned about versioning APIs which we don't consume ourselves - the few in lighthouse are the most pertinent. When we consume our own APIs we can usually make any required changes in the calling services. I prefer the structure from the stackoverflow post (using routes.py etc.) in the lighthouse issue, what did you think of that structure?
crawler/__init__.py
Outdated
| from crawler.blueprints.v1 import cherrypicker_test_data as cptd_v1 | ||
|
|
||
| app.register_blueprint(cptd_v1.bp, url_prefix="/v1") | ||
| app.register_blueprint(cptd_v1.bp) # Also serve v1 at the root of the host for now |
There was a problem hiding this comment.
I think it's worth making the change in lighthouse now to prevent this TODO kicking around for too long.
There was a problem hiding this comment.
Will do ... I just need to make that update in another branch then this will be coming out immediately
| yield process | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("json", [{}, {"run_id": None}]) | ||
| def test_generate_endpoint_invalid_json(json, client, logger_messages): | ||
| response = client.post("/cherrypick-test-data", json=json) | ||
| @pytest.mark.parametrize("endpoint_path", ENDPOINT_PATHS) |
There was a problem hiding this comment.
Does this post to both root and v1 endpoints?
There was a problem hiding this comment.
Yes they both get tested. Of course they should be identical, but I will be removing the root endpoint as soon as I've changed Lighthouse and I'll strip this back to just the v1 endpoint in the test.
Thanks @harrietc52 . I think I'll not rename it at this stage. It's been |
Closes: #394
Decided it was best to start with the Crawler API since it's much smaller and lets us focus on the architecture of the solution. The V1 blueprint is the only one at the moment and gets served at both
https://crawler-url.sanger/cherrypick-test-dataand also athttps://crawler-url.sanger/v1/cherrypick-test-data.In practice I will update the URL being used by Lighthouse and then I'll drop the unversioned endpoint.
When adding a
/v2/API, the common logic for the existing endpoint can go in another module that isn't decorated. Each of the two blueprints (v1 and v2) can then call that common module to keep things clean. If the new endpoint is significantly different, then the logic flow can be directed as needed from these blueprints.