Conversation
pickypg
left a comment
pickypg
left a comment
There was a problem hiding this comment.
LGTM with some questions about the added comment lines and maybe an improvement we should add to limit trying to access those APIs in 9.0+?
|
@pickypg I left those as comments for the moment, as it would be best if there's a way to modify the API call to force a response. I'm flipping this to be a draft for the moment, as I research to see if there's any way around this. |
|
@mmahacek It's possible that those APIs require a special request header, which we have a branch of code ready to support. I hope to revisit that next week (not the first time I've had that hope though). My main point is, for your research, see if those APIs need a request header and, if so, we probably have a solution. |
|
It does look like if we can include |
|
With the changes, we'll able to support both scenarios and other headers. |
|
Do you think we can merge this PR to include these fixes, and leave the issue open pending a second PR to resolve the headers at some point in the future? |
|
I'm going to try to use this to pressure myself to get the header logic merged next week, then we can adjust these API calls to work. That timing work out okay? I would be unlikely to do a release before next week anyway. |
|
That's completely fine with me. I wasn't even expecting this fast a response to the review request, so thank you! |
|
@pickypg I fixed the merge conflict, though I don't have have access to hit the merge button. |
|
You're all good @mmahacek. I am still trying to carve out time to do what I said earlier in #904 (comment) |
2 participants
Partially addresses #890 to fix endpoints where the URL has changed with version 9.0.0+
Checklist