Skip to content

Pydantic response models#44

Merged
CasperWA merged 2 commits intomasterfrom
fix_issue06_pydantic_response_models
Feb 25, 2022
Merged

Pydantic response models#44
CasperWA merged 2 commits intomasterfrom
fix_issue06_pydantic_response_models

Conversation

@CasperWA
Copy link
Copy Markdown
Contributor

@CasperWA CasperWA commented Feb 9, 2022

Closes #9
Closes #51
Closes #47

The latest commits expect the new SessionUpdate object is returned from strategy methods.

Note: This is a PR opened by me from the work done by @pscff to implement pydantic data models for the API responses and more. I have done a full overhaul in the latter commits to fully finish and implement the ideas started by @pscff.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Feb 9, 2022

Codecov Report

Merging #44 (5e678ed) into master (3e57cd0) will decrease coverage by 0.92%.
The diff coverage is 62.89%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #44      +/-   ##
==========================================
- Coverage   69.49%   68.57%   -0.93%     
==========================================
  Files           8       15       +7     
  Lines         295      420     +125     
==========================================
+ Hits          205      288      +83     
- Misses         90      132      +42     
Flag Coverage Δ
pytest 68.57% <62.89%> (-0.93%) ⬇️

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

Impacted Files Coverage Δ
app/routers/dataresource.py 29.41% <19.29%> (-3.38%) ⬇️
app/routers/session.py 50.74% <43.18%> (-6.67%) ⬇️
app/main.py 78.37% <57.14%> (+2.70%) ⬆️
app/routers/transformation.py 68.65% <63.63%> (-13.17%) ⬇️
app/routers/datafilter.py 80.85% <73.52%> (-11.26%) ⬇️
app/routers/mapping.py 80.85% <75.67%> (-11.26%) ⬇️
app/models/error.py 85.71% <85.71%> (ø)
app/models/response.py 92.85% <92.85%> (ø)
app/models/datafilter.py 100.00% <100.00%> (ø)
app/models/dataresource.py 100.00% <100.00%> (ø)
... and 5 more

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 3e57cd0...5e678ed. Read the comment docs.

@CasperWA CasperWA marked this pull request as ready for review February 22, 2022 18:31
@CasperWA
Copy link
Copy Markdown
Contributor Author

CasperWA commented Feb 22, 2022

@pscff I'll stop messing around here now - feel free to comment on whether you want to implement further changes (or simply do) in the scope of this PR :)

@CasperWA
Copy link
Copy Markdown
Contributor Author

@quaat and @jesper-friis There is some confusion on my side as to the dataresource endpoint, when handling a download and parse strategy. For the get() method it seems straight forward, but at the moment parse strategies' initialize() method was never called. I've implemented this here now, but I'm unsure about the exact nature of this - please review.
In my opinion it would be cleaner if only a parse strategy was invoked, but that would implicitly mean all parse strategies should make sure to invoke a download strategy internally; this doesn't seem reasonable. But then should the parse strategy be initialized first or the download strategy? Hmm, bah.

@CasperWA CasperWA removed the request for review from jesper-friis February 23, 2022 07:52
Comment thread app/routers/dataresource.py
Comment thread app/routers/dataresource.py
Comment thread app/routers/dataresource.py
This was referenced Feb 25, 2022
pscff and others added 2 commits February 25, 2022 13:35
Add package for defining custom pydantic response-models
Include isort, black in dev requiremetns (see git-hooks).

Add HTTPValidationError response model.
Include 4xx response models at router-level.

Use starlette's status codes.

Raise an error when session_id cannot be found in the cache.

Add Status and Error responses to all endpoints.

Use a generic Status Response Model.

Move response helper functions to models.response.

Add response models to datafilter, mapping, and session.
Fixed app.main for updated routers.

Update to oteapi-core v0.1.0.

Remove TYPE_CHECKING blocks from code coverage.

Change `main._APP` to `main.APP`.

Use status codes via `fastapi.status`.

Remove dependencies from git hooks as these will be installed and
handled by pre-commit.
@CasperWA CasperWA force-pushed the fix_issue06_pydantic_response_models branch from 709d52c to f7851a4 Compare February 25, 2022 12:40
@CasperWA CasperWA merged commit f7851a4 into master Feb 25, 2022
@CasperWA CasperWA deleted the fix_issue06_pydantic_response_models branch February 25, 2022 12:41
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.

Update to oteapi-core v0.1.0 Handle new SessionUpdate response from strategy methods Use pydantic models as Response Model

4 participants