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

warn about staus before checking content type #35

Merged
merged 1 commit into from
Mar 31, 2015

Conversation

cpsievert
Copy link
Contributor

No description provided.

@cpsievert
Copy link
Contributor Author

This should fix #27 (I think)

@sckott
Copy link
Contributor

sckott commented Mar 31, 2015

i think the travis fail is fine, i think it shouldn't work on PRs since we use secure env var for testing

@sckott
Copy link
Contributor

sckott commented Mar 31, 2015

i think you mean #28 ?

@sckott
Copy link
Contributor

sckott commented Mar 31, 2015

i think with #27 we get that error because we have warn_for_status(), so a 405 is thrown, but process continues, then the header check is done, and that's a stop function

sckott added a commit that referenced this pull request Mar 31, 2015
warn about staus before checking content type
@sckott sckott merged commit c3d2698 into ropensci:master Mar 31, 2015
@cpsievert
Copy link
Contributor Author

Hey Scott :)

When I uploaded large files, I was getting: Error: x$headers$content-type== "application/json; charset=utf-8" is not TRUE

This should now at least give an informative warning before that error: Warning message: In process(response) : client error: (405) Method Not Allowed

So, this probably isn't a complete solution, but at least a little closer. Keep up the great work! I may be sending you more PRs in the future :)

@sckott
Copy link
Contributor

sckott commented Mar 31, 2015

Right, sounds good.

Still working on this 405 problem. It doesn't even seem like the right http error code, continue work on this in #27

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.

2 participants