163 yaml datasets #284
163 yaml datasets #284
Conversation
nschoenbrot
changed the title
|
@seanpreston It looks like I will be working on this ticket again during this sprint. For this ticket how do we want to handle documentation for the YAML endpoints? We could just leave it out of the Open API spec, or start manually writing the Open API spec? See: https://fastapi.tiangolo.com/advanced/extending-openapi/ including @conceptualshark since there is a documentation question here. |
Great suggestion. Let's ship this as a v0, then follow-up with manually adding the OpenAPI spec as you've suggested as part of a subsequent ticket. |
Since this endpoint needs to access the request, the open api spec will not be generated correctly. To include this, extend open api: https://fastapi.tiangolo.com/advanced/extending-openapi/
nschoenbrot
force-pushed
the
163_Yaml_Datasets
branch
from
d423b5a to
cc64433
Compare
…o the yaml dataset endpoint.
nschoenbrot
force-pushed
the
163_Yaml_Datasets
branch
from
cc64433 to
3adbb2a
Compare
nschoenbrot
force-pushed
the
163_Yaml_Datasets
branch
from
7209f00 to
281d451
Compare
…Update Postgres Dataset YAML
| "script": { | ||
| "exec": [ | ||
| "pm.collectionVariables.set(\"client_token\", pm.response.json()[\"access_token\"])" | ||
| ], |
There was a problem hiding this comment.
This script will take the access token from the response and update the collection variable. That way we don't need to manually copy and paste.
| return BulkPutDataset( | ||
| succeeded=created_or_updated, | ||
| failed=failed, | ||
| ) | ||
|
|
||
|
|
||
| def create_or_update_dataset( |
There was a problem hiding this comment.
This function was extracted from the Patch dataset function. So that both dataset PATCHing endpoints (JSON and YAML) can call the same underlying code.
nschoenbrot
changed the title
| response_model=BulkPutDataset, | ||
| include_in_schema=False # Not including this path in the schema. | ||
| # Since this yaml function needs to access the request, the open api spec will not be generated correctly. | ||
| # To include this path, extend open api: https://fastapi.tiangolo.com/advanced/extending-openapi/ |
There was a problem hiding this comment.
This is fine for now but we should find a way to expose this information in our Swagger API UI if we add additional YAML endpoints. Can we document this endpoint somewhere if we haven't already?
There was a problem hiding this comment.
I brought this up with @seanpreston & @conceptualshark. We can have a subsequent ticket.
I would think our solution would depend on how many resources and actions we will support in the yaml API. For me there are open questions:
- How much will the JSON and YAML APIs diverge?
- Is it going to be a large API? If so we might want to find a tool to autogenerate open api spec.
- Do we want a separate spec for the yaml api?
| @@ -68,6 +69,9 @@ | |||
| DATASETS = CONNECTION_BY_KEY + "/dataset" | |||
| DATASET_BY_KEY = CONNECTION_BY_KEY + "/dataset/{fides_key}" | |||
|
|
|||
| # YAML Collection URLs | |||
| YAML_DATASETS = YAML + DATASETS | |||
There was a problem hiding this comment.
Nit: This will result in the URL /api/v1/yml/connection/{key}/dataset — could we append YAML here instead of prepend for the result /api/v1/connection/{key}/dataset/yml/, to reflect the hierarchy of what's being executed.
* 163 Initial commit showing that yaml dataset can be saved. * 163 Refactor the yaml dataset endpoint and added pytests. * 163 Exclude yaml dataset endpoint from open api spec. Since this endpoint needs to access the request, the open api spec will not be generated correctly. To include this, extend open api: https://fastapi.tiangolo.com/advanced/extending-openapi/ * 163 Formatted with Black. * 163 Fixes for mypy. * 163 Added happy path test for patching yaml dataset. * 163 respond with 415 if the wrong content type is specified a patch to the yaml dataset endpoint. * 163 Respond with 400 when the dataset yaml cannot be loaded. * 163 Added example of a YAML PATCH request for dataset. Named: Create/Update Postgres Dataset YAML * 163 Added example of a YAML PATCH request for dataset. Named: Create/Update Postgres Dataset YAML * 163 Test patching multiple datasets at a time. * 163 Return specific yaml error in http response. * 163 Ran make black. * 163 Trying to get linter to pass. Co-authored-by: Nelson Schoenbrot <nelson@ethyca.com>
Purpose
Allow datasets to be patched using YAML in place of JSON.
Changes
-Creates a YAML path for patching datasets. Since FastAPI does not have built in support for YAML in the way it does for JSON, the YAML endpoint will directly access the request object and deserialize the body. But the JSON and YAML paths will still hit the same underlying code. (I wanted to try and route based on content type so that the same path could be shared, from what I found content type based routing has not be built into FastAPI yet)
-Was able to get a successful response from the YAML Datasets endpoint locally.
-TODO Add more tests to cover edge cases such as bad content type, bad YAML file, multiple items listed under the YAMLdataset.-TODO Fix type checking issues found by mypy. There are typing issues around the deserialized request body.-TODO Update postman collection.-TODO Come up with a strategy on how to generate custom OpenAPI spec for YAML endpoints.Checklist
Run Unsafe PR Checkslabel has been applied, and checks have passed, if this PR touches any external servicesTicket
Fixes #163