Conversation
fostimus
left a comment
fostimus
left a comment
There was a problem hiding this comment.
In the original issue, it was about improving request_rest, while this stands up a new method. Should this be baked into request_rest so consumers benefit just from upgrading, or is this potentially breaking and should have people migrate to this new method?
| ] | ||
|
|
||
| def get_url(self): | ||
| return "" # Not used since we expect full URLs |
There was a problem hiding this comment.
need to define this? the base of this func throws an error, prob can keep if we don't want people to use this
There was a problem hiding this comment.
this is intentionally defined as an empty string, otherwise request_rest tries to append whatever URL is passed in to whatever is returned in get_url
There was a problem hiding this comment.
if that's the case then yeah def, but doesn't this function get inherited from here? I'm not a python guy so idk how inheritance works here
There was a problem hiding this comment.
it does get inherited from there but we are overriding it here by returning an empty string
| if should_raise: | ||
| with pytest.raises( | ||
| ValueError, | ||
| match="URL must start with one of: https://app.useanvil.com", |
There was a problem hiding this comment.
ideally this uses VALID_HOSTS somehow
| if should_raise: | ||
| with pytest.raises( | ||
| ValueError, | ||
| match="URL must start with one of: https://app.useanvil.com", |
There was a problem hiding this comment.
ideally this uses VALID_HOSTS somehow
| api = RestRequest(self.client, options=options) | ||
| return api | ||
|
|
||
| def request_fully_qualified(self, options: Optional[dict] = None): |
There was a problem hiding this comment.
idt this is actually used anywhere - looks like the func above is used here. a couple other placess too, probably should use this new version
There was a problem hiding this comment.
Yeah, request_rest also doesnt seem to be called anywhere. I guess someone could use these if they wanted to when making other requests to Anvil
3 participants
Closes #79
Description
This PR addresses Issue #79 where passing a fully-qualified URL to request_rest.get() or request_rest.post() would incorrectly prepend the base URL, resulting in an invalid request.
Changes