Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Switch URL_PREFIX for SENTRY_URL_PREFIX #1

Closed
wants to merge 1 commit into from

3 participants

@ghickman

In the case where URL_PREFIX doesn't exist, use SENTRY_URL_PREFIX in
it's place since this should be defined in the user's local settings.

@ghickman ghickman Use SENTRY_URL_PREFIX instead of URL_PREFIX incase.
In the case where URL_PREFIX doesn't exist, use SENTRY_URL_PREFIX in
it's place since this should be defined in the user's local settings.
ea3653f
@mattrobenolt

In what case would URL_PREFIX not exist? I took that logic out of the actual email module. I haven't seen SENTRY_URL_PREFIX.

@ghickman

URL_PREFIX is defined as an empty string here: https://github.com/dcramer/sentry/blob/3.8.2/sentry/conf/defaults.py#L41

However I found it was turning up as None a lot of the time. I never actually tracked down where this was getting changed as I knew the SENTRY_URL_PREFIX was set in my config. I figured that as it's part of the generated config file and mentioned in the docs (http://sentry.readthedocs.org/en/latest/quickstart/index.html#initializing-the-configuration) it was safe to rely on this.

@mattrobenolt

Fair enough. I'll look into it tomorrow and track down the best options. Thanks.

@mattrobenolt

All SENTRY_* variables are internally stripped of SENTRY_, see: https://github.com/dcramer/sentry/blob/master/sentry/conf/settings.py#L23-25

There shouldn't be an instance where SENTRY_URL_PREFIX exists without URL_PREFIX existing unless you've manually overwritten somewhere.

What are you doing in your code to not have a URL_PREFIX?

@ghickman

This is only true if you're using sentry as app inside a django project.

When installed as a system application the runner specifies the server settings file which doesn't contain the conversion code.

I could change the code to try SENTRY_URL_PREFIX first then URL_PREFIX next?

@mattrobenolt

Do you mind sending me your project so I can reproduce? I don't feel comfortable changing this because no other plugin uses that convention. They all access URL_PREFIX directly. Maybe when using the runner, you're supposed to declare URL_PREFIX instead?

/cc @dcramer

@mattrobenolt

I feel like, if anything, this could be an oversight in how Sentry handles loading different configs. One is being passed through the parser which converts SENTRY_* and the other isn't.

@dcramer

Ya this is the same setting. You configure things using SENTRY_*, but they're not referenced that way internally.

@mattrobenolt

Thanks for the confirmation. :)

@ghickman If there is still an issue with Sentry, you should file a bug there.

@dcramer

Oh actually, the problem is this is importing Django settings, and it should be doing "from sentry.conf import settings"

@ghickman ghickman referenced this pull request from a commit in ghickman/sentry-groveio
@ghickman ghickman Import settings from sentry instead of django
Fixes URL_PREFIX not existing on settings if SENTRY_URL_PREFIX has been
used in the settings.
Pointed out by @dcramer in #1
9d0d2fd
@mattrobenolt

Ahh. I see. Thanks. :)

@ghickman ghickman referenced this pull request from a commit in ghickman/sentry-groveio
@ghickman ghickman Import settings from sentry instead of django
Fixes URL_PREFIX not existing on settings if SENTRY_URL_PREFIX has been
used in the settings.
Pointed out by @dcramer in #1
e346458
@ghickman ghickman referenced this pull request from a commit in ghickman/sentry-groveio
@ghickman ghickman Import settings from sentry instead of django
Fixes URL_PREFIX not existing on settings if SENTRY_URL_PREFIX has been
used in the settings.
Pointed out by @dcramer in #1
4861cd5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 14, 2012
  1. @ghickman

    Use SENTRY_URL_PREFIX instead of URL_PREFIX incase.

    ghickman authored
    In the case where URL_PREFIX doesn't exist, use SENTRY_URL_PREFIX in
    it's place since this should be defined in the user's local settings.
This page is out of date. Refresh to see the latest.
Showing with 1 addition and 1 deletion.
  1. +1 −1  sentry_groveio/models.py
View
2  sentry_groveio/models.py
@@ -54,7 +54,7 @@ def send_payload(self, token, service, event, group, message):
values = {
'service': service,
'message': message,
- 'url': '%s/%d/group/%d/' % (settings.URL_PREFIX, group.project_id, group.id),
+ 'url': '%s/%d/group/%d/' % (settings.SENTRY_URL_PREFIX, group.project_id, group.id),
'icon_url': self.get_option('icon_url', event.project)
}
Something went wrong with that request. Please try again.