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

Improve error message when issue data structure is unexpected. #55

Merged
merged 1 commit into from
Feb 11, 2019

Conversation

sminnee
Copy link
Contributor

@sminnee sminnee commented Jan 22, 2019

Provides more diagnostics for #54.

@sminnee
Copy link
Contributor Author

sminnee commented Jan 30, 2019

@luandy64 could we merge this and push to StitchData.com?

@luandy64
Copy link
Contributor

I don't think this is the right approach for the problem.

We can use requests's raise_for_status() and get the response you are looking for. I encourage you to change authed_get_all_pages() in the tap to call raise_for_status() just before it yields.

@luandy64 luandy64 added enhancement help wanted Anyone is free to try and tackle this issue and removed help wanted Anyone is free to try and tackle this issue labels Jan 31, 2019
@luandy64
Copy link
Contributor

Also just to clarify, have you been running the tap locally and testing things?

@sminnee
Copy link
Contributor Author

sminnee commented Feb 9, 2019

I've generally been running the tap locally. However, I am unable to replicate this error situation on my local environment.

@sminnee
Copy link
Contributor Author

sminnee commented Feb 9, 2019

Right, yes, I see now that the response created is a 502.

@sminnee sminnee closed this Feb 9, 2019
@sminnee
Copy link
Contributor Author

sminnee commented Feb 9, 2019

I have amended the PR as recommended.

@luandy64 luandy64 merged commit 884869d into singer-io:master Feb 11, 2019
AJWurts pushed a commit to villagelabsco/tap-github that referenced this pull request Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants