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

Implement dataSink and dataSource API. #17

Merged

Conversation

csadorf
Copy link
Collaborator

@csadorf csadorf commented Jun 7, 2022

Implements all dataSink and dataSource-related operations in compliance with the OpenStack Swift object storage API on the container level. That means the operations for creating, replacing, and deleting datasets (objects) are in compliance with the API specified by Swift and it is assumed that one dataSink/dataSource is equivalent to a object storage container.

@csadorf csadorf force-pushed the implement-datasink-and-datasource-api-as-object-storage-api branch 2 times, most recently from b48c3f1 to 7e4a29f Compare June 7, 2022 17:39
@csadorf csadorf force-pushed the implement-datasink-and-datasource-api-as-object-storage-api branch from 035d8f9 to 8aafc68 Compare June 7, 2022 17:47
@csadorf csadorf marked this pull request as ready for review June 7, 2022 17:49
@csadorf
Copy link
Collaborator Author

csadorf commented Jun 7, 2022

@yoavnash @pablo-de-andres This is my proposal for the dataSink and dataSource API. It is based on the OpenStack SWIFT API which is largely compatible with S3 for core functions. I only specified operations for the purpose of MarketPlace that appeared most important to me.

Finally, you may notice that some functions have "optional path parameters". To my knowledge neither FastAPI nor Flask support optional path parameters since they conceptually represent different endpoints. However, you can associate
multiple endpoints with one response handler function – which I did. In that case the corresponding function parameter must be optional which FastAPI picks up and then annoyingly represents as an optional query parameter for the endpoint without path parameter. I think that's a tolerable nuisance for the sake of implementational simplicity. I primarily wanted to alert you of that, but please feel free to disagree with my assessment.

marketplace_standard_app_api/main.py Show resolved Hide resolved
marketplace_standard_app_api/main.py Outdated Show resolved Hide resolved
marketplace_standard_app_api/main.py Outdated Show resolved Hide resolved
@yoavnash
Copy link
Member

yoavnash commented Jun 9, 2022

In addition to the above, it seems that capabilities for querying (queryDataset, queryCollection) are also planned to be dropped. I wonder how much they are needed.

csadorf and others added 2 commits June 10, 2022 10:04
Co-authored-by: nash <62147380+yoavnash@users.noreply.github.com>
@csadorf
Copy link
Collaborator Author

csadorf commented Jun 10, 2022

In addition to the above, it seems that capabilities for querying (queryDataset, queryCollection) are also planned to be dropped. I wonder how much they are needed.

I don't plan to drop anything. I am just introducing the objectStorage API here. Capabilities for querying can be added later.

@csadorf
Copy link
Collaborator Author

csadorf commented Jun 13, 2022

@yoavnash @pablo-de-andres As discussed, I will implement the container-level API. After I'm done I will re-request your review.

@csadorf
Copy link
Collaborator Author

csadorf commented Jun 13, 2022

@yoavnash @pablo-de-andres Ready for another review iteration.

Copy link
Member

@yoavnash yoavnash left a comment

Choose a reason for hiding this comment

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

Sorry for the delay. I think that looks very good.

@csadorf csadorf merged commit 37fb59b into main Jun 17, 2022
@csadorf csadorf deleted the implement-datasink-and-datasource-api-as-object-storage-api branch June 17, 2022 11:03
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.

Revise the dataSource and dataSink endpoints to match those of standard object stores
3 participants