chore: clearly state system.url-prefix value should be set#3100
chore: clearly state system.url-prefix value should be set#3100aldy505 wants to merge 7 commits intogetsentry:masterfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3100 +/- ##
==========================================
- Coverage 99.01% 98.02% -0.99%
==========================================
Files 3 3
Lines 203 203
==========================================
- Hits 201 199 -2
- Misses 2 4 +2 ☔ View full report in Codecov by Sentry. |
|
This should be handled by the initial set up screen, that's why we didn't ask it to be set in the config. Agree with docs update but disagree with this specific change. If there's an issue with the set-up flow we should address that instead (on get sentry/sentry side) |
I'm quite pessimistic on having that changed on the sentry/sentry side, since this is a common issue that most people encountered during their first time self-hosting sentry. |
|
I'm mostly trying to understand how this is not being set in the initial setup. Do we have any links to people asking about this so I can check? |
A few that I remembered: |
|
Sorry for the late turnaround here. How about we try to do //github.com//issues/2621#issuecomment-1842014431 and the subsequent comment instead? Hard-coding makes changing this a bit more tricky (although I agree that it should not be something that is changed regularly) |
I don't really get what you're trying to do here.. since I believe modifying through the web app is something that's not persisted on disk (to postgres or file). But again, I'm not so sure about that, hence I prefer it to be done via configuration file. Don't get me wrong: I wanted the modified configurations to survive between updates, but I don't know whether it will correctly survive that. |
It should persist on Postgres, or somewhere persistent as that's how the options system works. I'll list the relevant code for reference:
Now in number 1 and 3, you can see that it will be disabled on the UI when it was set from the disk (options file). That said I cought something quite interesting: looks like we fetch the default value from WDYT? |
Putting on the @hubertdeng123 @azaslavsky what's your thoughts about this? |
I feel like
|
hubertdeng123
left a comment
There was a problem hiding this comment.
Have you tested the setting SENTRY_SYSTEM_URL_PREFIX will work after registering the option system.url-prefix? My concern here is someone manages to register the option for system.url-prefix changing the SENTRY_SYSTEM_URL_PREFIX will not do anything and it will instead cause confusion.
|
I too echo what @BYK has mentioned above. The option should be set on the initial set up screen. Personally I have not seen DSN appearing as empty as a reproducible issue and setting the |
There are more than one cases of people having their DSN or email links empty, most of it because
system.url-prefixis not set on the config. Hope this will solves that (by providing a clearer message). But, I guess we should add this one to the docs site too, regarding "which config file you should be modifying before you run your sentry".Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.