kedro-plugins
kedro-plugins copied to clipboard
kedro-org
feat(datasets): add wrapped dataset to store the response from a POST/PUT request
Proof of concept to start discussions about design on how to store and use the response object.
Description
To pursue discussions on how we could store the response object from a POST or PUT api request.
Relates to https://github.com/kedro-org/kedro-plugins/issues/748
Development notes
- Only poc code
- Tests to be implemented
Checklist
- [x] Opened this PR as a 'Draft Pull Request' if it is work-in-progress
- [ ] Updated the documentation to reflect the code changes
- [ ] Added a description of this change in the relevant
RELEASE.mdfile - [ ] Added tests to cover my changes
- [ ] Received approvals from at least half of the TSC (required for adding a new, non-experimental dataset)
No worries. Thx for getting back to me!
Yes, I chose the wrapped dataset mainly because it would avoid maintenance burden by relying on the implementation of the wrapped dataset rather than duplicating the logic there.
Of course, it's a subjective opinion, so feel free to challenge this, if it doesn't follow kedro's guidelines.
I don't think it's a breaking change of the API dataset: the wrapped dataset args are optionals.
If not provided, the response is not saved as it's already the case.
It's been a while since I looked at the issue this PR is for, but if I understand correctly the problem with APIDataset is that it only returns the response, but doesn't actually save it? And this PR aims to add a wrapper so a user can indicate what format to save the response in?
On the original issue I said I didn't like the idea of importing other datasets, which I still agree with, but at the same time it does make sense now I see it in code 😜 I'll have to think about this a bit more to review it properly.
It's been a while since I looked at the issue this PR is for, but if I understand correctly the problem with APIDataset is that it only returns the response, but doesn't actually save it? And this PR aims to add a wrapper so a user can indicate what format to save the response in?
On the original issue I said I didn't like the idea of importing other datasets, which I still agree with, but at the same time it does make sense now I see it in code 😜 I'll have to think about this a bit more to review it properly.
Yes that's right:
- some APIs would return a response that might be important to store somewhere,
- yes exactly.
Good to know, thx! (I'm a bit biased because we almost always use wrapped dataset for our custom datasets: we're lazy and like relying on a maintained classe :-) )
The idea of having such a dataset makes sense to me. Implementation-wise, I would expect to see something similar to https://docs.kedro.org/projects/kedro-datasets/en/kedro-datasets-6.0.0/_modules/kedro_datasets/partitions/partitioned_dataset.html#PartitionedDataset, so logic of requesting data is separated from saving it via dataset and the extension is derived from dataset set rather than vice a versa.
Hi team and @npfp, this PR has been open for a while. I'll add the related issue #748 to the backlog and maybe we can pick this PR up again after discussion?
We will take over this PR and finish the implementation - @ankatiyar to provide further details (https://github.com/kedro-org/kedro-plugins/issues/748)
