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

Issue 3662: HTTP request source support 'csv type' response #3663

Conversation

dmateusp
Copy link
Contributor

What

Resolves #3662

How

It implements the second solution described in the issue, if the response received by the HTTP Request Source is a list of lists, then it wraps the lists into {"data": <list>} records

Pre-merge Checklist

  • Run integration tests
  • Publish Docker images

Recommended reading order

  1. test.java
  2. component.ts
  3. the rest

Copy link
Contributor

@sherifnada sherifnada left a comment

Choose a reason for hiding this comment

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

thanks @dmateusp ! one change requested and we are good to go

) -> Generator[AirbyteMessage, None, None]:
r = self._make_request(config)
if r.status_code != 200:
raise Exception(f"Request failed. {r.text}")

# need to eagerly fetch the json.
data = r.json()

if isinstance(data, list) and isinstance(data[0], list):
Copy link
Contributor

Choose a reason for hiding this comment

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

we should also verify len(data) > 0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hey @sherifnada, I'm actually going to close this PR
I agree with @marcosmarxm that the responses from the US Census API are so unconventional that they would be better supported to a separate "Source"

@dmateusp dmateusp closed this May 28, 2021
tkorenko added a commit that referenced this pull request Dec 16, 2022
+ Changes 'deploy-docs-site' workflow and 'deploy_docusaurus'
  script so these two things are used for doc PR verification
  as well.
tkorenko added a commit that referenced this pull request Dec 16, 2022
+ Changes 'deploy-docs-site' workflow and 'deploy_docusaurus'
  script so these two things are used for doc PR verification
  as well.
tkorenko added a commit that referenced this pull request Dec 16, 2022
+ In "Docs PR check mode" stop before "yarn deploy"
tkorenko added a commit that referenced this pull request Dec 16, 2022
+ In "Docs PR check mode" stop before "yarn deploy"
tkorenko added a commit that referenced this pull request Dec 16, 2022
+ Routes Slack notifications to '#docs'
+ Sets default value of SKIP_DEPLOY variable in deploy_docusaurus
  script
tkorenko added a commit that referenced this pull request Dec 16, 2022
+ Routes Slack notifications to '#docs'
+ Sets default value of SKIP_DEPLOY variable in deploy_docusaurus
  script
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.

Add a US Census connector
3 participants