ref(app-platform): Backfill IntegrationFeature table by MeredithAnya · Pull Request #13548 · getsentry/sentry

MeredithAnya · 2019-06-05T20:00:17Z

In order for the Feature Gating to work, we need each sentry app integration to have at least one feature listed (see why here).

This PR does two things to make that happen:

backfills the sentry apps that have already been created to have the default feature integrations-api. Nothing uses this right now but will be used in ref(app-platform): Add integration features to SentryAppDetailsModal #13393
adds creating an IntegrationFeature into the sentry apps Creator

We will also need to backfill specifically the two published apps we have to have the appropriate features.

MeredithAnya · 2019-06-05T22:07:48Z

@JTCunning @mattrobenoltIntegrationFeature has no records - and SentryApp has a very small amount of records

mattrobenolt · 2019-06-05T22:23:07Z

+ def _create_integration_feature(self):
+ # sentry apps must have at least one feature
+ # defaults to 'api'
+ IntegrationFeature.objects.create(


This isn't caring about duplicates, is it? Isn't that less than ideal if there were more than 1 IntegrationFeature paired with a SentryApp?

well you can have up to four types of features associated with a sentry app, but yeah we should make it so you can't add the same type twice - that should probably be added to the model right?

Yeah, does that mean you'd have a UNIQUE on (sentry_app, feature) ?

Yeah will need to add that

@mattrobenolt #13573

mnoble

Small logger change.

mnoble · 2019-06-07T21:03:07Z

+ sentry_app=self.sentry_app,
+ )
+ except IntegrityError as e:
+ logger.info(


You can just use self.log(extra={...}) here (https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/mediator.py#L183).
Each mediator already has a logger and this function will add a bunch of other data, if available. It also makes sure the logger names are consistent.

mattrobenolt · 2019-06-07T22:44:01Z

+ def _create_integration_feature(self):
+ # sentry apps must have at least one feature
+ # defaults to 'integrations-api'
+ try:


This is the wrong pattern. It also needs to be wrapped ina transaction block.
try: withtransaction.atomic(): IntegrationFeature.objects.create(...) exceptIntegrityError: pass
And do you really need to log when this happens? This behavior is more like, "ensure this exists, but fail silently" kinda thing when we do it elsewhere. We never log this.

This whole object's logic is run inside of a transaction. Every Mediators' is. https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/mediator.py#L156-L164

The whole mediator is wrapped in a transaction, so is it necessary to have it within that specific block too?
Also since this is in the creator, there really shouldn't be one that already exists - so if that does happen maybe I'd want to know?

The whole mediator is wrapped in a transaction, so is it necessary to have it within that specific block too?
Yes, it still needs a transaction. Technically a checkpoint. Otherwise, it'll break your other transaction and put it in an invalid state.
Also since this is in the creator, there really shouldn't be one that already exists - so if that does happen maybe I'd want to know?
That's your call, and just depends or your expectations.

* master: (102 commits) fix(symbolicator): Tests are no longer allowed failures (#13566) fix(yarn): Fix yarn path in distutils (#13647) ref(ui): Remove OrganizationState from a bunch of places (#13646) ref(ui): Remove crufty global exports (#13578) feat(events-v2): Page loader improvements (#13645) fix: Query the right set of issues when given no set of envs for a given first release (#13551) feat(ui): update incidents icon (#13622) fix(ui): remove rate-limited values from graph when no events are present (#13624) feat(events-v2) Add a event graph to the event modal (#13568) fix(store): Check project.status is VISIBLE before attempting to process perf(snuba): Limit SnubaEvent.get_event to return the first result only. (#13644) tests(percy): Fix Incident status item dynamic time (#13640) ref(sentry10): Clean up action creators (#13636) ref(app-platform): Backfill IntegrationFeature table (#13548) ref(utils): Remove default / reexports from utils.jsx (#13581) feat(sentry10): Remove the ability to set a default environment (#13637) feat(api): Add url unfurls for incidents to slack integration (SEN-566) build(yarn): Upgrade yarn to 1.16.0, embed in repo (#13569) feat(api): Allow passing group_ids to `OrganizationGroupIndexEndpoint` for bulk fetching groups (SEN-701) feat(ui): Adds related issues to Incidents sidebar [SEN-695] (#13617) ...

MeredithAnya · 2019-06-05T20:00:17Z

In order for the Feature Gating to work, we need each sentry app integration to have at least one feature listed (see why here).

This PR does two things to make that happen:

backfills the sentry apps that have already been created to have the default feature integrations-api. Nothing uses this right now but will be used in ref(app-platform): Add integration features to SentryAppDetailsModal #13393
adds creating an IntegrationFeature into the sentry apps Creator

We will also need to backfill specifically the two published apps we have to have the appropriate features.

MeredithAnya · 2019-06-05T22:07:48Z

@JTCunning @mattrobenoltIntegrationFeature has no records - and SentryApp has a very small amount of records

mattrobenolt · 2019-06-05T22:23:07Z

+ def _create_integration_feature(self):
+ # sentry apps must have at least one feature
+ # defaults to 'api'
+ IntegrationFeature.objects.create(


This isn't caring about duplicates, is it? Isn't that less than ideal if there were more than 1 IntegrationFeature paired with a SentryApp?

well you can have up to four types of features associated with a sentry app, but yeah we should make it so you can't add the same type twice - that should probably be added to the model right?

Yeah, does that mean you'd have a UNIQUE on (sentry_app, feature) ?

Yeah will need to add that

@mattrobenolt #13573

mnoble

Small logger change.

mnoble · 2019-06-07T21:03:07Z

+ sentry_app=self.sentry_app,
+ )
+ except IntegrityError as e:
+ logger.info(


You can just use self.log(extra={...}) here (https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/mediator.py#L183).
Each mediator already has a logger and this function will add a bunch of other data, if available. It also makes sure the logger names are consistent.

mattrobenolt · 2019-06-07T22:44:01Z

+ def _create_integration_feature(self):
+ # sentry apps must have at least one feature
+ # defaults to 'integrations-api'
+ try:


This is the wrong pattern. It also needs to be wrapped ina transaction block.
try: withtransaction.atomic(): IntegrationFeature.objects.create(...) exceptIntegrityError: pass
And do you really need to log when this happens? This behavior is more like, "ensure this exists, but fail silently" kinda thing when we do it elsewhere. We never log this.

This whole object's logic is run inside of a transaction. Every Mediators' is. https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/mediator.py#L156-L164

The whole mediator is wrapped in a transaction, so is it necessary to have it within that specific block too?
Also since this is in the creator, there really shouldn't be one that already exists - so if that does happen maybe I'd want to know?

The whole mediator is wrapped in a transaction, so is it necessary to have it within that specific block too?
Yes, it still needs a transaction. Technically a checkpoint. Otherwise, it'll break your other transaction and put it in an invalid state.
Also since this is in the creator, there really shouldn't be one that already exists - so if that does happen maybe I'd want to know?
That's your call, and just depends or your expectations.

* master: (102 commits) fix(symbolicator): Tests are no longer allowed failures (#13566) fix(yarn): Fix yarn path in distutils (#13647) ref(ui): Remove OrganizationState from a bunch of places (#13646) ref(ui): Remove crufty global exports (#13578) feat(events-v2): Page loader improvements (#13645) fix: Query the right set of issues when given no set of envs for a given first release (#13551) feat(ui): update incidents icon (#13622) fix(ui): remove rate-limited values from graph when no events are present (#13624) feat(events-v2) Add a event graph to the event modal (#13568) fix(store): Check project.status is VISIBLE before attempting to process perf(snuba): Limit SnubaEvent.get_event to return the first result only. (#13644) tests(percy): Fix Incident status item dynamic time (#13640) ref(sentry10): Clean up action creators (#13636) ref(app-platform): Backfill IntegrationFeature table (#13548) ref(utils): Remove default / reexports from utils.jsx (#13581) feat(sentry10): Remove the ability to set a default environment (#13637) feat(api): Add url unfurls for incidents to slack integration (SEN-566) build(yarn): Upgrade yarn to 1.16.0, embed in repo (#13569) feat(api): Allow passing group_ids to `OrganizationGroupIndexEndpoint` for bulk fetching groups (SEN-701) feat(ui): Adds related issues to Incidents sidebar [SEN-695] (#13617) ...

MeredithAnya · 2019-06-05T20:00:17Z

In order for the Feature Gating to work, we need each sentry app integration to have at least one feature listed (see why here).

This PR does two things to make that happen:

backfills the sentry apps that have already been created to have the default feature integrations-api. Nothing uses this right now but will be used in ref(app-platform): Add integration features to SentryAppDetailsModal #13393
adds creating an IntegrationFeature into the sentry apps Creator

We will also need to backfill specifically the two published apps we have to have the appropriate features.

MeredithAnya · 2019-06-05T22:07:48Z

@JTCunning @mattrobenoltIntegrationFeature has no records - and SentryApp has a very small amount of records

mattrobenolt · 2019-06-05T22:23:07Z

+ def _create_integration_feature(self):
+ # sentry apps must have at least one feature
+ # defaults to 'api'
+ IntegrationFeature.objects.create(


This isn't caring about duplicates, is it? Isn't that less than ideal if there were more than 1 IntegrationFeature paired with a SentryApp?

well you can have up to four types of features associated with a sentry app, but yeah we should make it so you can't add the same type twice - that should probably be added to the model right?

Yeah, does that mean you'd have a UNIQUE on (sentry_app, feature) ?

Yeah will need to add that

@mattrobenolt #13573

mnoble

Small logger change.

mnoble · 2019-06-07T21:03:07Z

+ sentry_app=self.sentry_app,
+ )
+ except IntegrityError as e:
+ logger.info(


You can just use self.log(extra={...}) here (https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/mediator.py#L183).
Each mediator already has a logger and this function will add a bunch of other data, if available. It also makes sure the logger names are consistent.

mattrobenolt · 2019-06-07T22:44:01Z

+ def _create_integration_feature(self):
+ # sentry apps must have at least one feature
+ # defaults to 'integrations-api'
+ try:


This is the wrong pattern. It also needs to be wrapped ina transaction block.
try: withtransaction.atomic(): IntegrationFeature.objects.create(...) exceptIntegrityError: pass
And do you really need to log when this happens? This behavior is more like, "ensure this exists, but fail silently" kinda thing when we do it elsewhere. We never log this.

This whole object's logic is run inside of a transaction. Every Mediators' is. https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/mediator.py#L156-L164

The whole mediator is wrapped in a transaction, so is it necessary to have it within that specific block too?
Also since this is in the creator, there really shouldn't be one that already exists - so if that does happen maybe I'd want to know?

The whole mediator is wrapped in a transaction, so is it necessary to have it within that specific block too?
Yes, it still needs a transaction. Technically a checkpoint. Otherwise, it'll break your other transaction and put it in an invalid state.
Also since this is in the creator, there really shouldn't be one that already exists - so if that does happen maybe I'd want to know?
That's your call, and just depends or your expectations.

* master: (102 commits) fix(symbolicator): Tests are no longer allowed failures (#13566) fix(yarn): Fix yarn path in distutils (#13647) ref(ui): Remove OrganizationState from a bunch of places (#13646) ref(ui): Remove crufty global exports (#13578) feat(events-v2): Page loader improvements (#13645) fix: Query the right set of issues when given no set of envs for a given first release (#13551) feat(ui): update incidents icon (#13622) fix(ui): remove rate-limited values from graph when no events are present (#13624) feat(events-v2) Add a event graph to the event modal (#13568) fix(store): Check project.status is VISIBLE before attempting to process perf(snuba): Limit SnubaEvent.get_event to return the first result only. (#13644) tests(percy): Fix Incident status item dynamic time (#13640) ref(sentry10): Clean up action creators (#13636) ref(app-platform): Backfill IntegrationFeature table (#13548) ref(utils): Remove default / reexports from utils.jsx (#13581) feat(sentry10): Remove the ability to set a default environment (#13637) feat(api): Add url unfurls for incidents to slack integration (SEN-566) build(yarn): Upgrade yarn to 1.16.0, embed in repo (#13569) feat(api): Allow passing group_ids to `OrganizationGroupIndexEndpoint` for bulk fetching groups (SEN-701) feat(ui): Adds related issues to Incidents sidebar [SEN-695] (#13617) ...

MeredithAnya · 2019-06-05T20:00:17Z

In order for the Feature Gating to work, we need each sentry app integration to have at least one feature listed (see why here).

This PR does two things to make that happen:

backfills the sentry apps that have already been created to have the default feature integrations-api. Nothing uses this right now but will be used in ref(app-platform): Add integration features to SentryAppDetailsModal #13393
adds creating an IntegrationFeature into the sentry apps Creator

We will also need to backfill specifically the two published apps we have to have the appropriate features.

MeredithAnya · 2019-06-05T22:07:48Z

@JTCunning @mattrobenoltIntegrationFeature has no records - and SentryApp has a very small amount of records

mattrobenolt · 2019-06-05T22:23:07Z

+ def _create_integration_feature(self):
+ # sentry apps must have at least one feature
+ # defaults to 'api'
+ IntegrationFeature.objects.create(


This isn't caring about duplicates, is it? Isn't that less than ideal if there were more than 1 IntegrationFeature paired with a SentryApp?

well you can have up to four types of features associated with a sentry app, but yeah we should make it so you can't add the same type twice - that should probably be added to the model right?

Yeah, does that mean you'd have a UNIQUE on (sentry_app, feature) ?

Yeah will need to add that

@mattrobenolt #13573

mnoble

Small logger change.

mnoble · 2019-06-07T21:03:07Z

+ sentry_app=self.sentry_app,
+ )
+ except IntegrityError as e:
+ logger.info(


You can just use self.log(extra={...}) here (https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/mediator.py#L183).
Each mediator already has a logger and this function will add a bunch of other data, if available. It also makes sure the logger names are consistent.

mattrobenolt · 2019-06-07T22:44:01Z

+ def _create_integration_feature(self):
+ # sentry apps must have at least one feature
+ # defaults to 'integrations-api'
+ try:


This is the wrong pattern. It also needs to be wrapped ina transaction block.
try: withtransaction.atomic(): IntegrationFeature.objects.create(...) exceptIntegrityError: pass
And do you really need to log when this happens? This behavior is more like, "ensure this exists, but fail silently" kinda thing when we do it elsewhere. We never log this.

This whole object's logic is run inside of a transaction. Every Mediators' is. https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/mediator.py#L156-L164

The whole mediator is wrapped in a transaction, so is it necessary to have it within that specific block too?
Also since this is in the creator, there really shouldn't be one that already exists - so if that does happen maybe I'd want to know?

The whole mediator is wrapped in a transaction, so is it necessary to have it within that specific block too?
Yes, it still needs a transaction. Technically a checkpoint. Otherwise, it'll break your other transaction and put it in an invalid state.
Also since this is in the creator, there really shouldn't be one that already exists - so if that does happen maybe I'd want to know?
That's your call, and just depends or your expectations.

* master: (102 commits) fix(symbolicator): Tests are no longer allowed failures (#13566) fix(yarn): Fix yarn path in distutils (#13647) ref(ui): Remove OrganizationState from a bunch of places (#13646) ref(ui): Remove crufty global exports (#13578) feat(events-v2): Page loader improvements (#13645) fix: Query the right set of issues when given no set of envs for a given first release (#13551) feat(ui): update incidents icon (#13622) fix(ui): remove rate-limited values from graph when no events are present (#13624) feat(events-v2) Add a event graph to the event modal (#13568) fix(store): Check project.status is VISIBLE before attempting to process perf(snuba): Limit SnubaEvent.get_event to return the first result only. (#13644) tests(percy): Fix Incident status item dynamic time (#13640) ref(sentry10): Clean up action creators (#13636) ref(app-platform): Backfill IntegrationFeature table (#13548) ref(utils): Remove default / reexports from utils.jsx (#13581) feat(sentry10): Remove the ability to set a default environment (#13637) feat(api): Add url unfurls for incidents to slack integration (SEN-566) build(yarn): Upgrade yarn to 1.16.0, embed in repo (#13569) feat(api): Allow passing group_ids to `OrganizationGroupIndexEndpoint` for bulk fetching groups (SEN-701) feat(ui): Adds related issues to Incidents sidebar [SEN-695] (#13617) ...

MeredithAnya · 2019-06-05T20:00:17Z

In order for the Feature Gating to work, we need each sentry app integration to have at least one feature listed (see why here).

This PR does two things to make that happen:

backfills the sentry apps that have already been created to have the default feature integrations-api. Nothing uses this right now but will be used in ref(app-platform): Add integration features to SentryAppDetailsModal #13393
adds creating an IntegrationFeature into the sentry apps Creator

We will also need to backfill specifically the two published apps we have to have the appropriate features.

MeredithAnya · 2019-06-05T22:07:48Z

@JTCunning @mattrobenoltIntegrationFeature has no records - and SentryApp has a very small amount of records

mattrobenolt · 2019-06-05T22:23:07Z

+ def _create_integration_feature(self):
+ # sentry apps must have at least one feature
+ # defaults to 'api'
+ IntegrationFeature.objects.create(


This isn't caring about duplicates, is it? Isn't that less than ideal if there were more than 1 IntegrationFeature paired with a SentryApp?

well you can have up to four types of features associated with a sentry app, but yeah we should make it so you can't add the same type twice - that should probably be added to the model right?

Yeah, does that mean you'd have a UNIQUE on (sentry_app, feature) ?

Yeah will need to add that

@mattrobenolt #13573

mnoble

Small logger change.

mnoble · 2019-06-07T21:03:07Z

+ sentry_app=self.sentry_app,
+ )
+ except IntegrityError as e:
+ logger.info(


You can just use self.log(extra={...}) here (https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/mediator.py#L183).
Each mediator already has a logger and this function will add a bunch of other data, if available. It also makes sure the logger names are consistent.

mattrobenolt · 2019-06-07T22:44:01Z

+ def _create_integration_feature(self):
+ # sentry apps must have at least one feature
+ # defaults to 'integrations-api'
+ try:


This is the wrong pattern. It also needs to be wrapped ina transaction block.
try: withtransaction.atomic(): IntegrationFeature.objects.create(...) exceptIntegrityError: pass
And do you really need to log when this happens? This behavior is more like, "ensure this exists, but fail silently" kinda thing when we do it elsewhere. We never log this.

This whole object's logic is run inside of a transaction. Every Mediators' is. https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/mediator.py#L156-L164

The whole mediator is wrapped in a transaction, so is it necessary to have it within that specific block too?
Also since this is in the creator, there really shouldn't be one that already exists - so if that does happen maybe I'd want to know?

The whole mediator is wrapped in a transaction, so is it necessary to have it within that specific block too?
Yes, it still needs a transaction. Technically a checkpoint. Otherwise, it'll break your other transaction and put it in an invalid state.
Also since this is in the creator, there really shouldn't be one that already exists - so if that does happen maybe I'd want to know?
That's your call, and just depends or your expectations.

* master: (102 commits) fix(symbolicator): Tests are no longer allowed failures (#13566) fix(yarn): Fix yarn path in distutils (#13647) ref(ui): Remove OrganizationState from a bunch of places (#13646) ref(ui): Remove crufty global exports (#13578) feat(events-v2): Page loader improvements (#13645) fix: Query the right set of issues when given no set of envs for a given first release (#13551) feat(ui): update incidents icon (#13622) fix(ui): remove rate-limited values from graph when no events are present (#13624) feat(events-v2) Add a event graph to the event modal (#13568) fix(store): Check project.status is VISIBLE before attempting to process perf(snuba): Limit SnubaEvent.get_event to return the first result only. (#13644) tests(percy): Fix Incident status item dynamic time (#13640) ref(sentry10): Clean up action creators (#13636) ref(app-platform): Backfill IntegrationFeature table (#13548) ref(utils): Remove default / reexports from utils.jsx (#13581) feat(sentry10): Remove the ability to set a default environment (#13637) feat(api): Add url unfurls for incidents to slack integration (SEN-566) build(yarn): Upgrade yarn to 1.16.0, embed in repo (#13569) feat(api): Allow passing group_ids to `OrganizationGroupIndexEndpoint` for bulk fetching groups (SEN-701) feat(ui): Adds related issues to Incidents sidebar [SEN-695] (#13617) ...

MeredithAnya · 2019-06-05T20:00:17Z

In order for the Feature Gating to work, we need each sentry app integration to have at least one feature listed (see why here).

This PR does two things to make that happen:

backfills the sentry apps that have already been created to have the default feature integrations-api. Nothing uses this right now but will be used in ref(app-platform): Add integration features to SentryAppDetailsModal #13393
adds creating an IntegrationFeature into the sentry apps Creator

We will also need to backfill specifically the two published apps we have to have the appropriate features.

MeredithAnya · 2019-06-05T22:07:48Z

@JTCunning @mattrobenoltIntegrationFeature has no records - and SentryApp has a very small amount of records

mattrobenolt · 2019-06-05T22:23:07Z

+ def _create_integration_feature(self):
+ # sentry apps must have at least one feature
+ # defaults to 'api'
+ IntegrationFeature.objects.create(


This isn't caring about duplicates, is it? Isn't that less than ideal if there were more than 1 IntegrationFeature paired with a SentryApp?

well you can have up to four types of features associated with a sentry app, but yeah we should make it so you can't add the same type twice - that should probably be added to the model right?

Yeah, does that mean you'd have a UNIQUE on (sentry_app, feature) ?

Yeah will need to add that

@mattrobenolt #13573

mnoble

Small logger change.

mnoble · 2019-06-07T21:03:07Z

+ sentry_app=self.sentry_app,
+ )
+ except IntegrityError as e:
+ logger.info(


You can just use self.log(extra={...}) here (https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/mediator.py#L183).
Each mediator already has a logger and this function will add a bunch of other data, if available. It also makes sure the logger names are consistent.

mattrobenolt · 2019-06-07T22:44:01Z

+ def _create_integration_feature(self):
+ # sentry apps must have at least one feature
+ # defaults to 'integrations-api'
+ try:


This is the wrong pattern. It also needs to be wrapped ina transaction block.
try: withtransaction.atomic(): IntegrationFeature.objects.create(...) exceptIntegrityError: pass
And do you really need to log when this happens? This behavior is more like, "ensure this exists, but fail silently" kinda thing when we do it elsewhere. We never log this.

This whole object's logic is run inside of a transaction. Every Mediators' is. https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/mediator.py#L156-L164

The whole mediator is wrapped in a transaction, so is it necessary to have it within that specific block too?
Also since this is in the creator, there really shouldn't be one that already exists - so if that does happen maybe I'd want to know?

The whole mediator is wrapped in a transaction, so is it necessary to have it within that specific block too?
Yes, it still needs a transaction. Technically a checkpoint. Otherwise, it'll break your other transaction and put it in an invalid state.
Also since this is in the creator, there really shouldn't be one that already exists - so if that does happen maybe I'd want to know?
That's your call, and just depends or your expectations.

* master: (102 commits) fix(symbolicator): Tests are no longer allowed failures (#13566) fix(yarn): Fix yarn path in distutils (#13647) ref(ui): Remove OrganizationState from a bunch of places (#13646) ref(ui): Remove crufty global exports (#13578) feat(events-v2): Page loader improvements (#13645) fix: Query the right set of issues when given no set of envs for a given first release (#13551) feat(ui): update incidents icon (#13622) fix(ui): remove rate-limited values from graph when no events are present (#13624) feat(events-v2) Add a event graph to the event modal (#13568) fix(store): Check project.status is VISIBLE before attempting to process perf(snuba): Limit SnubaEvent.get_event to return the first result only. (#13644) tests(percy): Fix Incident status item dynamic time (#13640) ref(sentry10): Clean up action creators (#13636) ref(app-platform): Backfill IntegrationFeature table (#13548) ref(utils): Remove default / reexports from utils.jsx (#13581) feat(sentry10): Remove the ability to set a default environment (#13637) feat(api): Add url unfurls for incidents to slack integration (SEN-566) build(yarn): Upgrade yarn to 1.16.0, embed in repo (#13569) feat(api): Allow passing group_ids to `OrganizationGroupIndexEndpoint` for bulk fetching groups (SEN-701) feat(ui): Adds related issues to Incidents sidebar [SEN-695] (#13617) ...

MeredithAnya · 2019-06-05T20:00:17Z

In order for the Feature Gating to work, we need each sentry app integration to have at least one feature listed (see why here).

This PR does two things to make that happen:

backfills the sentry apps that have already been created to have the default feature integrations-api. Nothing uses this right now but will be used in ref(app-platform): Add integration features to SentryAppDetailsModal #13393
adds creating an IntegrationFeature into the sentry apps Creator

We will also need to backfill specifically the two published apps we have to have the appropriate features.

MeredithAnya · 2019-06-05T22:07:48Z

@JTCunning @mattrobenoltIntegrationFeature has no records - and SentryApp has a very small amount of records

mattrobenolt · 2019-06-05T22:23:07Z

+ def _create_integration_feature(self):
+ # sentry apps must have at least one feature
+ # defaults to 'api'
+ IntegrationFeature.objects.create(


This isn't caring about duplicates, is it? Isn't that less than ideal if there were more than 1 IntegrationFeature paired with a SentryApp?

well you can have up to four types of features associated with a sentry app, but yeah we should make it so you can't add the same type twice - that should probably be added to the model right?

Yeah, does that mean you'd have a UNIQUE on (sentry_app, feature) ?

Yeah will need to add that

@mattrobenolt #13573

mnoble

Small logger change.

mnoble · 2019-06-07T21:03:07Z

+ sentry_app=self.sentry_app,
+ )
+ except IntegrityError as e:
+ logger.info(


You can just use self.log(extra={...}) here (https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/mediator.py#L183).
Each mediator already has a logger and this function will add a bunch of other data, if available. It also makes sure the logger names are consistent.

mattrobenolt · 2019-06-07T22:44:01Z

+ def _create_integration_feature(self):
+ # sentry apps must have at least one feature
+ # defaults to 'integrations-api'
+ try:


This is the wrong pattern. It also needs to be wrapped ina transaction block.
try: withtransaction.atomic(): IntegrationFeature.objects.create(...) exceptIntegrityError: pass
And do you really need to log when this happens? This behavior is more like, "ensure this exists, but fail silently" kinda thing when we do it elsewhere. We never log this.

This whole object's logic is run inside of a transaction. Every Mediators' is. https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/mediator.py#L156-L164

The whole mediator is wrapped in a transaction, so is it necessary to have it within that specific block too?
Also since this is in the creator, there really shouldn't be one that already exists - so if that does happen maybe I'd want to know?

The whole mediator is wrapped in a transaction, so is it necessary to have it within that specific block too?
Yes, it still needs a transaction. Technically a checkpoint. Otherwise, it'll break your other transaction and put it in an invalid state.
Also since this is in the creator, there really shouldn't be one that already exists - so if that does happen maybe I'd want to know?
That's your call, and just depends or your expectations.

* master: (102 commits) fix(symbolicator): Tests are no longer allowed failures (#13566) fix(yarn): Fix yarn path in distutils (#13647) ref(ui): Remove OrganizationState from a bunch of places (#13646) ref(ui): Remove crufty global exports (#13578) feat(events-v2): Page loader improvements (#13645) fix: Query the right set of issues when given no set of envs for a given first release (#13551) feat(ui): update incidents icon (#13622) fix(ui): remove rate-limited values from graph when no events are present (#13624) feat(events-v2) Add a event graph to the event modal (#13568) fix(store): Check project.status is VISIBLE before attempting to process perf(snuba): Limit SnubaEvent.get_event to return the first result only. (#13644) tests(percy): Fix Incident status item dynamic time (#13640) ref(sentry10): Clean up action creators (#13636) ref(app-platform): Backfill IntegrationFeature table (#13548) ref(utils): Remove default / reexports from utils.jsx (#13581) feat(sentry10): Remove the ability to set a default environment (#13637) feat(api): Add url unfurls for incidents to slack integration (SEN-566) build(yarn): Upgrade yarn to 1.16.0, embed in repo (#13569) feat(api): Allow passing group_ids to `OrganizationGroupIndexEndpoint` for bulk fetching groups (SEN-701) feat(ui): Adds related issues to Incidents sidebar [SEN-695] (#13617) ...

MeredithAnya · 2019-06-05T20:00:17Z

In order for the Feature Gating to work, we need each sentry app integration to have at least one feature listed (see why here).

This PR does two things to make that happen:

backfills the sentry apps that have already been created to have the default feature integrations-api. Nothing uses this right now but will be used in ref(app-platform): Add integration features to SentryAppDetailsModal #13393
adds creating an IntegrationFeature into the sentry apps Creator

We will also need to backfill specifically the two published apps we have to have the appropriate features.

MeredithAnya · 2019-06-05T22:07:48Z

@JTCunning @mattrobenoltIntegrationFeature has no records - and SentryApp has a very small amount of records

mattrobenolt · 2019-06-05T22:23:07Z

+ def _create_integration_feature(self):
+ # sentry apps must have at least one feature
+ # defaults to 'api'
+ IntegrationFeature.objects.create(


This isn't caring about duplicates, is it? Isn't that less than ideal if there were more than 1 IntegrationFeature paired with a SentryApp?

well you can have up to four types of features associated with a sentry app, but yeah we should make it so you can't add the same type twice - that should probably be added to the model right?

Yeah, does that mean you'd have a UNIQUE on (sentry_app, feature) ?

Yeah will need to add that

@mattrobenolt #13573

mnoble

Small logger change.

mnoble · 2019-06-07T21:03:07Z

+ sentry_app=self.sentry_app,
+ )
+ except IntegrityError as e:
+ logger.info(


You can just use self.log(extra={...}) here (https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/mediator.py#L183).
Each mediator already has a logger and this function will add a bunch of other data, if available. It also makes sure the logger names are consistent.

mattrobenolt · 2019-06-07T22:44:01Z

+ def _create_integration_feature(self):
+ # sentry apps must have at least one feature
+ # defaults to 'integrations-api'
+ try:


This is the wrong pattern. It also needs to be wrapped ina transaction block.
try: withtransaction.atomic(): IntegrationFeature.objects.create(...) exceptIntegrityError: pass
And do you really need to log when this happens? This behavior is more like, "ensure this exists, but fail silently" kinda thing when we do it elsewhere. We never log this.

This whole object's logic is run inside of a transaction. Every Mediators' is. https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/mediator.py#L156-L164

The whole mediator is wrapped in a transaction, so is it necessary to have it within that specific block too?
Also since this is in the creator, there really shouldn't be one that already exists - so if that does happen maybe I'd want to know?

The whole mediator is wrapped in a transaction, so is it necessary to have it within that specific block too?
Yes, it still needs a transaction. Technically a checkpoint. Otherwise, it'll break your other transaction and put it in an invalid state.
Also since this is in the creator, there really shouldn't be one that already exists - so if that does happen maybe I'd want to know?
That's your call, and just depends or your expectations.

* master: (102 commits) fix(symbolicator): Tests are no longer allowed failures (#13566) fix(yarn): Fix yarn path in distutils (#13647) ref(ui): Remove OrganizationState from a bunch of places (#13646) ref(ui): Remove crufty global exports (#13578) feat(events-v2): Page loader improvements (#13645) fix: Query the right set of issues when given no set of envs for a given first release (#13551) feat(ui): update incidents icon (#13622) fix(ui): remove rate-limited values from graph when no events are present (#13624) feat(events-v2) Add a event graph to the event modal (#13568) fix(store): Check project.status is VISIBLE before attempting to process perf(snuba): Limit SnubaEvent.get_event to return the first result only. (#13644) tests(percy): Fix Incident status item dynamic time (#13640) ref(sentry10): Clean up action creators (#13636) ref(app-platform): Backfill IntegrationFeature table (#13548) ref(utils): Remove default / reexports from utils.jsx (#13581) feat(sentry10): Remove the ability to set a default environment (#13637) feat(api): Add url unfurls for incidents to slack integration (SEN-566) build(yarn): Upgrade yarn to 1.16.0, embed in repo (#13569) feat(api): Allow passing group_ids to `OrganizationGroupIndexEndpoint` for bulk fetching groups (SEN-701) feat(ui): Adds related issues to Incidents sidebar [SEN-695] (#13617) ...

MeredithAnya requested a review from mnoble June 5, 2019 21:51

mattrobenolt reviewed Jun 5, 2019
View reviewed changes

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from bfcc4ca to b0f14cdCompare June 7, 2019 20:49

mnoble suggested changes Jun 7, 2019
View reviewed changes

mattrobenolt reviewed Jun 7, 2019
View reviewed changes

Comment threadsrc/sentry/south_migrations/0487_backfill_integrationfeatures.py Outdated

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from 563f18c to 0e4d6feCompare June 10, 2019 23:41

mnoble reviewed Jun 11, 2019
View reviewed changes

Comment threadsrc/sentry/mediators/sentry_apps/creator.py Outdated
Comment threadtests/sentry/models/test_integrationfeature.py Outdated

MeredithAnya added 10 commits June 11, 2019 10:44

ref(app-platform): Backfill integrationfeature table
fd24c56

words
69303e1

remove old number
68d6660

add logging
370bdad

fix test, logging refactor
dc5a2f6

fix other test lol
9daf115

wrap in transaction
375179c

in creator as well
d1cc744

another test :)
349aa4a

do self.log correctly
da31bb6

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from 0e4d6fe to da31bb6Compare June 11, 2019 17:51

mnoble approved these changes Jun 11, 2019
View reviewed changes

MeredithAnya merged commit 77ad480 into masterJun 11, 2019

MeredithAnya deleted the app-plaform/backfill-integrationfeatures branch June 11, 2019 18:53

github-actionsBot locked and limited conversation to collaborators Dec 20, 2020

MeredithAnya requested a review from mnoble June 5, 2019 21:51

mattrobenolt reviewed Jun 5, 2019
View reviewed changes

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from bfcc4ca to b0f14cdCompare June 7, 2019 20:49

mnoble suggested changes Jun 7, 2019
View reviewed changes

mattrobenolt reviewed Jun 7, 2019
View reviewed changes

Comment threadsrc/sentry/south_migrations/0487_backfill_integrationfeatures.py Outdated

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from 563f18c to 0e4d6feCompare June 10, 2019 23:41

mnoble reviewed Jun 11, 2019
View reviewed changes

Comment threadsrc/sentry/mediators/sentry_apps/creator.py Outdated
Comment threadtests/sentry/models/test_integrationfeature.py Outdated

MeredithAnya added 10 commits June 11, 2019 10:44

ref(app-platform): Backfill integrationfeature table
fd24c56

words
69303e1

remove old number
68d6660

add logging
370bdad

fix test, logging refactor
dc5a2f6

fix other test lol
9daf115

wrap in transaction
375179c

in creator as well
d1cc744

another test :)
349aa4a

do self.log correctly
da31bb6

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from 0e4d6fe to da31bb6Compare June 11, 2019 17:51

mnoble approved these changes Jun 11, 2019
View reviewed changes

MeredithAnya merged commit 77ad480 into masterJun 11, 2019

MeredithAnya deleted the app-plaform/backfill-integrationfeatures branch June 11, 2019 18:53

github-actionsBot locked and limited conversation to collaborators Dec 20, 2020

MeredithAnya requested a review from mnoble June 5, 2019 21:51

mattrobenolt reviewed Jun 5, 2019
View reviewed changes

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from bfcc4ca to b0f14cdCompare June 7, 2019 20:49

mnoble suggested changes Jun 7, 2019
View reviewed changes

mattrobenolt reviewed Jun 7, 2019
View reviewed changes

Comment threadsrc/sentry/south_migrations/0487_backfill_integrationfeatures.py Outdated

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from 563f18c to 0e4d6feCompare June 10, 2019 23:41

mnoble reviewed Jun 11, 2019
View reviewed changes

Comment threadsrc/sentry/mediators/sentry_apps/creator.py Outdated
Comment threadtests/sentry/models/test_integrationfeature.py Outdated

MeredithAnya added 10 commits June 11, 2019 10:44

ref(app-platform): Backfill integrationfeature table
fd24c56

words
69303e1

remove old number
68d6660

add logging
370bdad

fix test, logging refactor
dc5a2f6

fix other test lol
9daf115

wrap in transaction
375179c

in creator as well
d1cc744

another test :)
349aa4a

do self.log correctly
da31bb6

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from 0e4d6fe to da31bb6Compare June 11, 2019 17:51

mnoble approved these changes Jun 11, 2019
View reviewed changes

MeredithAnya merged commit 77ad480 into masterJun 11, 2019

MeredithAnya deleted the app-plaform/backfill-integrationfeatures branch June 11, 2019 18:53

github-actionsBot locked and limited conversation to collaborators Dec 20, 2020

MeredithAnya requested a review from mnoble June 5, 2019 21:51

mattrobenolt reviewed Jun 5, 2019
View reviewed changes

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from bfcc4ca to b0f14cdCompare June 7, 2019 20:49

mnoble suggested changes Jun 7, 2019
View reviewed changes

mattrobenolt reviewed Jun 7, 2019
View reviewed changes

Comment threadsrc/sentry/south_migrations/0487_backfill_integrationfeatures.py Outdated

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from 563f18c to 0e4d6feCompare June 10, 2019 23:41

mnoble reviewed Jun 11, 2019
View reviewed changes

Comment threadsrc/sentry/mediators/sentry_apps/creator.py Outdated
Comment threadtests/sentry/models/test_integrationfeature.py Outdated

MeredithAnya added 10 commits June 11, 2019 10:44

ref(app-platform): Backfill integrationfeature table
fd24c56

words
69303e1

remove old number
68d6660

add logging
370bdad

fix test, logging refactor
dc5a2f6

fix other test lol
9daf115

wrap in transaction
375179c

in creator as well
d1cc744

another test :)
349aa4a

do self.log correctly
da31bb6

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from 0e4d6fe to da31bb6Compare June 11, 2019 17:51

mnoble approved these changes Jun 11, 2019
View reviewed changes

MeredithAnya merged commit 77ad480 into masterJun 11, 2019

MeredithAnya deleted the app-plaform/backfill-integrationfeatures branch June 11, 2019 18:53

github-actionsBot locked and limited conversation to collaborators Dec 20, 2020

MeredithAnya requested a review from mnoble June 5, 2019 21:51

mattrobenolt reviewed Jun 5, 2019
View reviewed changes

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from bfcc4ca to b0f14cdCompare June 7, 2019 20:49

mnoble suggested changes Jun 7, 2019
View reviewed changes

mattrobenolt reviewed Jun 7, 2019
View reviewed changes

Comment threadsrc/sentry/south_migrations/0487_backfill_integrationfeatures.py Outdated

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from 563f18c to 0e4d6feCompare June 10, 2019 23:41

mnoble reviewed Jun 11, 2019
View reviewed changes

Comment threadsrc/sentry/mediators/sentry_apps/creator.py Outdated
Comment threadtests/sentry/models/test_integrationfeature.py Outdated

MeredithAnya added 10 commits June 11, 2019 10:44

ref(app-platform): Backfill integrationfeature table
fd24c56

words
69303e1

remove old number
68d6660

add logging
370bdad

fix test, logging refactor
dc5a2f6

fix other test lol
9daf115

wrap in transaction
375179c

in creator as well
d1cc744

another test :)
349aa4a

do self.log correctly
da31bb6

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from 0e4d6fe to da31bb6Compare June 11, 2019 17:51

mnoble approved these changes Jun 11, 2019
View reviewed changes

MeredithAnya merged commit 77ad480 into masterJun 11, 2019

MeredithAnya deleted the app-plaform/backfill-integrationfeatures branch June 11, 2019 18:53

github-actionsBot locked and limited conversation to collaborators Dec 20, 2020

MeredithAnya requested a review from mnoble June 5, 2019 21:51

mattrobenolt reviewed Jun 5, 2019
View reviewed changes

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from bfcc4ca to b0f14cdCompare June 7, 2019 20:49

mnoble suggested changes Jun 7, 2019
View reviewed changes

mattrobenolt reviewed Jun 7, 2019
View reviewed changes

Comment threadsrc/sentry/south_migrations/0487_backfill_integrationfeatures.py Outdated

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from 563f18c to 0e4d6feCompare June 10, 2019 23:41

mnoble reviewed Jun 11, 2019
View reviewed changes

Comment threadsrc/sentry/mediators/sentry_apps/creator.py Outdated
Comment threadtests/sentry/models/test_integrationfeature.py Outdated

MeredithAnya added 10 commits June 11, 2019 10:44

ref(app-platform): Backfill integrationfeature table
fd24c56

words
69303e1

remove old number
68d6660

add logging
370bdad

fix test, logging refactor
dc5a2f6

fix other test lol
9daf115

wrap in transaction
375179c

in creator as well
d1cc744

another test :)
349aa4a

do self.log correctly
da31bb6

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from 0e4d6fe to da31bb6Compare June 11, 2019 17:51

mnoble approved these changes Jun 11, 2019
View reviewed changes

MeredithAnya merged commit 77ad480 into masterJun 11, 2019

MeredithAnya deleted the app-plaform/backfill-integrationfeatures branch June 11, 2019 18:53

github-actionsBot locked and limited conversation to collaborators Dec 20, 2020

MeredithAnya requested a review from mnoble June 5, 2019 21:51

mattrobenolt reviewed Jun 5, 2019
View reviewed changes

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from bfcc4ca to b0f14cdCompare June 7, 2019 20:49

mnoble suggested changes Jun 7, 2019
View reviewed changes

mattrobenolt reviewed Jun 7, 2019
View reviewed changes

Comment threadsrc/sentry/south_migrations/0487_backfill_integrationfeatures.py Outdated

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from 563f18c to 0e4d6feCompare June 10, 2019 23:41

mnoble reviewed Jun 11, 2019
View reviewed changes

Comment threadsrc/sentry/mediators/sentry_apps/creator.py Outdated
Comment threadtests/sentry/models/test_integrationfeature.py Outdated

MeredithAnya added 10 commits June 11, 2019 10:44

ref(app-platform): Backfill integrationfeature table
fd24c56

words
69303e1

remove old number
68d6660

add logging
370bdad

fix test, logging refactor
dc5a2f6

fix other test lol
9daf115

wrap in transaction
375179c

in creator as well
d1cc744

another test :)
349aa4a

do self.log correctly
da31bb6

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from 0e4d6fe to da31bb6Compare June 11, 2019 17:51

mnoble approved these changes Jun 11, 2019
View reviewed changes

MeredithAnya merged commit 77ad480 into masterJun 11, 2019

MeredithAnya deleted the app-plaform/backfill-integrationfeatures branch June 11, 2019 18:53

github-actionsBot locked and limited conversation to collaborators Dec 20, 2020

MeredithAnya requested a review from mnoble June 5, 2019 21:51

mattrobenolt reviewed Jun 5, 2019
View reviewed changes

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from bfcc4ca to b0f14cdCompare June 7, 2019 20:49

mnoble suggested changes Jun 7, 2019
View reviewed changes

mattrobenolt reviewed Jun 7, 2019
View reviewed changes

Comment threadsrc/sentry/south_migrations/0487_backfill_integrationfeatures.py Outdated

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from 563f18c to 0e4d6feCompare June 10, 2019 23:41

mnoble reviewed Jun 11, 2019
View reviewed changes

Comment threadsrc/sentry/mediators/sentry_apps/creator.py Outdated
Comment threadtests/sentry/models/test_integrationfeature.py Outdated

MeredithAnya added 10 commits June 11, 2019 10:44

ref(app-platform): Backfill integrationfeature table
fd24c56

words
69303e1

remove old number
68d6660

add logging
370bdad

fix test, logging refactor
dc5a2f6

fix other test lol
9daf115

wrap in transaction
375179c

in creator as well
d1cc744

another test :)
349aa4a

do self.log correctly
da31bb6

MeredithAnya force-pushed the app-plaform/backfill-integrationfeatures branch from 0e4d6fe to da31bb6Compare June 11, 2019 17:51

mnoble approved these changes Jun 11, 2019
View reviewed changes

MeredithAnya merged commit 77ad480 into masterJun 11, 2019

MeredithAnya deleted the app-plaform/backfill-integrationfeatures branch June 11, 2019 18:53

github-actionsBot locked and limited conversation to collaborators Dec 20, 2020

Uh oh!

Uh oh!

Conversation

MeredithAnya commented Jun 5, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MeredithAnya commented Jun 5, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MeredithAnyaJun 6, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mnoble left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

MeredithAnya commented Jun 5, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MeredithAnya commented Jun 5, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MeredithAnyaJun 6, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mnoble left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

MeredithAnya commented Jun 5, 2019•
edited
Loading

MeredithAnya commented Jun 5, 2019•
edited
Loading

MeredithAnyaJun 6, 2019•
edited
Loading

mnoble left a comment •
edited
Loading

MeredithAnya commented Jun 5, 2019•
edited
Loading

MeredithAnya commented Jun 5, 2019•
edited
Loading

MeredithAnyaJun 6, 2019•
edited
Loading

mnoble left a comment •
edited
Loading

MeredithAnya commented Jun 5, 2019•
edited
Loading

MeredithAnya commented Jun 5, 2019•
edited
Loading

MeredithAnyaJun 6, 2019•
edited
Loading

mnoble left a comment •
edited
Loading