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

Decouple SimpleRetriever and HttpStream classes #14012

Closed
Tracked by #19658
girarda opened this issue Jun 22, 2022 · 1 comment
Closed
Tracked by #19658

Decouple SimpleRetriever and HttpStream classes #14012

girarda opened this issue Jun 22, 2022 · 1 comment
Labels
CDK Connector Development Kit CDKv2 team/extensibility technical-debt issues to fix code smell type/enhancement New feature or request

Comments

@girarda
Copy link
Contributor

girarda commented Jun 22, 2022

Tell us about the problem you're trying to solve

The SimpleRetriever class extends HttpStream, which allows us to easily reuse a lot of the non-trivial mechanics related to sending HTTP requests, pagination, and error extraction. This however comes at the cost of losing flexibility, both in terms of control flow, and in terms of how we can send requests as HttpStream does not support other protocols.

Describe the solution you’d like

I'd like to decouple the 2 classes, and move the Http specific logic to the HttpRequester so the retriever can focus exclusively on orchestrating the requester, stream_slicer, and extractor.

Note that low code connectors will still need to use the HttpAvailabilityStrategy by default. That should probably be set in the HttpRequester's config.

Describe the alternative you’ve considered or used

A clear and concise description of any alternative solutions or features you've considered or are using today.

Additional context

Add any other context or screenshots about the feature request here.

Are you willing to submit a PR?

Remove this with your answer :-)

@girarda girarda added type/enhancement New feature or request CDK Connector Development Kit CDKv2 team/extensibility labels Jun 22, 2022
@sherifnada sherifnada mentioned this issue Nov 9, 2022
10 tasks
@girarda girarda mentioned this issue Nov 21, 2022
49 tasks
@girarda girarda added the technical-debt issues to fix code smell label Dec 22, 2022
@girarda
Copy link
Contributor Author

girarda commented Oct 11, 2023

this is done thanks @flash1293 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CDK Connector Development Kit CDKv2 team/extensibility technical-debt issues to fix code smell type/enhancement New feature or request
Projects
Status: Done
Development

No branches or pull requests

1 participant