[IMP] webservice: combine the url with collection's url #36
Conversation
|
Hi @etobella, |
gurneyalex
force-pushed
the
16.0_webservice_collection_url
branch
2 times, most recently
from
6ba8448 to
9bc0dc6
Compare
| url = self.collection.url | ||
| elif not url.startswith(self.collection.url): | ||
| if not url.startswith("http"): | ||
| url = self.collection.url + url |
There was a problem hiding this comment.
| url = self.collection.url + url | |
| url = f"{self.collection.url.rstrip('/')/{url.lstrip('/')" |
There was a problem hiding this comment.
Question: why would you use this in favor of url params?
There was a problem hiding this comment.
currently it is possible to use to send http requests on a different url (different name even) => I'm not fond of this but I don't want to break code that could use this "feature".
In my case, I have a number of endpoints to access which are on "https://some.service.provider/api-prefix" and I find it nicer to use call() with only the part of the URL which is after this bit. That first part of the URL is going to be different in integration and production, but the value which is passed to call() is going to be stable.
But maybe I'm missing soemthing obvious?
There was a problem hiding this comment.
but that's exactly what url_params are for...
The base url will be different by env: fine, you have the env conf for it.
You will have something like https://sendto.me/prefix/{endpoint}/suffix and you can pass endpoint=foo as arg.
That's the idea. However this is probably a nice improvement for when the endpoint is always at the end of the URL.
There was a problem hiding this comment.
@simahawk can you update your review?
|
do we need an update on the readme? |
when a call is made with just a path, combine it with the collection's url better combination of urls (Co-authored-by: Simone Orsi <[email protected]>):
gurneyalex
force-pushed
the
16.0_webservice_collection_url
branch
from
0d974bb to
9057e53
Compare
|
/ocabot merge minor |
|
On my way to merge this fine PR! |
|
Congratulations, your PR was merged at c28decf. Thanks a lot for contributing to OCA. ❤️ |
when a call is made with just a path, combine it with the collection's url