ref(api): Use relative endpoint for ChunkUploadEndpoint if upload-url-prefix is absent #29347
Conversation
…-prefix is absent
|
|
||
| url = reverse("sentry-api-0-chunk-upload", args=[organization.slug]) | ||
| endpoint = urljoin(endpoint.rstrip("/") + "/", url.lstrip("/")) | ||
| if len(endpoint) == 0: |
There was a problem hiding this comment.
IIUC system.upload-url-prefix is registered with None as default value:
sentry/src/sentry/options/defaults.py
Line 31 in c4247cd
This means this len() will raise a TypeError if the option is not set. I think the pythonic way to write this is if not endpoint:
There was a problem hiding this comment.
It's been written like this for 4 years now, so I didn't want to change this, as I don't know the history of this code.
There was a problem hiding this comment.
oh right, that if condition was already there. hum. i think this is weird and we somehow got lucky so far. but i guess we have some confidence this won't blow up in prod so i don't mind 😄
|
|
||
| url = reverse("sentry-api-0-chunk-upload", args=[organization.slug]) | ||
| endpoint = urljoin(endpoint.rstrip("/") + "/", url.lstrip("/")) | ||
| if len(endpoint) == 0: |
There was a problem hiding this comment.
oh right, that if condition was already there. hum. i think this is weird and we somehow got lucky so far. but i guess we have some confidence this won't blow up in prod so i don't mind 😄
kamilogorek
changed the title
Making this change allows people to use sentry-cli configured URL for chunks upload, instead of relying on fixed URL configured in the on-premise instance.
sentry-cli handles relative URLs nicely already, so there are no changes needed there: https://github.com/getsentry/sentry-cli/blob/master/src/api.rs#L395-L405
It won't break anything sentry-side, as we check for prefix existence and add it if necessary here: https://github.com/getsentry/sentry/blob/master/src/sentry/api/client.py#L39-L42
I was trying to use
reverse()here to get an endpoint prefix, but I cannot make it work. If anyone has a better idea of how to make/api/0prefix generic in this function, I'm all ears.Fixes getsentry/sentry-cli#839
Closes getsentry/sentry-cli#862