feat(crons): Write new monitor GUIDs to monitor slug #45258
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
evanpurkhiser
requested review from
rjo100 and
a team
and removed request for
a team
github-actions
bot
added
the
Scope: Backend
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #45258 +/- ##
==========================================
+ Coverage 80.21% 80.23% +0.01%
==========================================
Files 4725 4725
Lines 198809 198815 +6
Branches 12022 12022
==========================================
+ Hits 159480 159512 +32
+ Misses 39067 39041 -26
Partials 262 262
|
rjo100
reviewed
Mar 1, 2023
| # NOTE: We ONLY set a slug while saving when creating a new monitor and | ||
| # the slug has not been set. Otherwise existing monitors without slugs | ||
| # would have their guids changed | ||
| if self._state.adding is True and self.slug is None: |
There was a problem hiding this comment.
since this is temporary, OK to use this hack
rjo100
approved these changes
Mar 1, 2023
| # the slug has not been set. Otherwise existing monitors without slugs | ||
| # would have their guids changed | ||
| if self._state.adding is True and self.slug is None: | ||
| self.guid = uuid4() |
There was a problem hiding this comment.
we have to generate a guid here because auto_add doesn't create it until later, right?
There was a problem hiding this comment.
Yes correct, unfortunately that happens in the super save call.
evanpurkhiser
force-pushed
the
evanpurkhiser/feat-monitors-write-new-monitor-guids-to-monitor-slug
branch
from
6bce6ed to
d2b6f06
Compare
rjo100
reviewed
Mar 1, 2023
src/sentry/models/monitor.py
Outdated
Comment on lines
161
to
163
| rv = super().save(*args, **kwargs) | ||
|
|
||
| return rv |
There was a problem hiding this comment.
could also just return this without the variable
There was a problem hiding this comment.
good catch, I think I copied this somewhere 👍
evanpurkhiser
force-pushed
the
evanpurkhiser/feat-monitors-write-new-monitor-guids-to-monitor-slug
branch
from
d2b6f06 to
e6cf4a3
Compare
evanpurkhiser
deleted the
evanpurkhiser/feat-monitors-write-new-monitor-guids-to-monitor-slug
branch
evanpurkhiser
changed the title
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For now slugs will simply match the GUIDs until we allow this to be API
configurable