Skip to content

Onboarding backfill#2750

Merged
ehfeng merged 11 commits into
masterfrom
onboarding-backfill
Feb 26, 2016
Merged

Onboarding backfill#2750
ehfeng merged 11 commits into
masterfrom
onboarding-backfill

Conversation

@ehfeng

@ehfeng ehfeng commented Feb 25, 2016

Copy link
Copy Markdown
Contributor

No description provided.

@codecov-io

Copy link
Copy Markdown

Current coverage is 83.47%

Merging #2750 into master will decrease coverage by -0.01% as of bb92895

@@            master   #2750   diff @@
======================================
  Files          882     882       
  Stmts        33741   33741       
  Branches         0       0       
  Methods          0       0       
======================================
- Hit          28168   28165     -3
  Partial          0       0       
- Missed        5573    5576     +3

Review entire Coverage Diff as of bb92895


Uncovered Suggestions

  1. +0.07% via ...try/utils/apidocs.py#432...454
  2. +0.07% via ...try/utils/apidocs.py#117...139
  3. +0.06% via ...gs/sentry_helpers.py#191...211
  4. See 7 more...

Powered by Codecov. Updated on successful CI builds.

@ehfeng

ehfeng commented Feb 25, 2016

Copy link
Copy Markdown
Contributor Author

local migration didn't work as I think I messed up my database state.

)

projects = Project.objects.filter(organization=org).exclude(first_event=None).order_by('first_event')
project_count = projects.count()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is relatively expensive if there were a large number of projects.

Really what you're asking for here is "are there 0, 1, or 1+" projects. You don't care about the exact count.

In that case, you should add a LIMIT clause in here with [0:2]. This will apply LIMIT 2. Now you can continue the same comparison with a much cheaper COUNT query.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see, you need to ultimately iterate over all the projects anyways later. So now this is being done in two queries. There's no reason to query for COUNT, when we can just evaluate the queryset and get the len() of the list.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@mattrobenolt

Copy link
Copy Markdown
Contributor

@ehfeng

from sentry.plugins import plugins
from sentry.plugins import IssueTrackingPlugin, NotificationPlugin

notification_keys = []
issue_keys = []

for p in plugins.all(version=1):
    if isinstance(p, NotificationPlugin):
        notification_keys.append(p._get_option_key('enabled'))
    elif isinstance(p, IssueTrackingPlugin):
        issue_keys.append(p._get_option_key('enabled'))

This will give you all the keys you need for checking against, rather than hardcoding a list.

ehfeng added a commit that referenced this pull request Feb 26, 2016
organization on-boarding data backfill
@ehfeng ehfeng merged commit 3c444b1 into master Feb 26, 2016
@ehfeng ehfeng deleted the onboarding-backfill branch March 1, 2016 01:23
@github-actions github-actions Bot locked and limited conversation to collaborators Dec 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants