Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(integrations): Make sync_status_inbound a Task #28906

Merged
merged 3 commits into from
Oct 4, 2021

Conversation

mgaeta
Copy link
Contributor

@mgaeta mgaeta commented Sep 28, 2021

Azure Devops webhooks requests are timing out waiting for statuses to sync. This PR makes syncing async.

Something to note I have to pass an arbitrary JSON blob to the task but I'm not sure if that's kosher.

@mgaeta mgaeta requested a review from a team September 28, 2021 17:49
@mgaeta mgaeta force-pushed the feat/api-2029-webhook-task-5 branch 2 times, most recently from 32f9299 to f0ac860 Compare October 1, 2021 18:58
Base automatically changed from feat/api-2029-webhook-task-5 to master October 1, 2021 19:27
Copy link
Member

@leeandher leeandher left a comment

Choose a reason for hiding this comment

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

👍 looks good

Comment on lines 332 to 334
def should_sync(self, attribute: str) -> bool:
key = getattr(self, f"{attribute}_key", None)
return self.org_integration.config.get(key, False)
Copy link
Member

Choose a reason for hiding this comment

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

this is a clever and satisfying refactor 👏


def get_done_states(self, project: str) -> Sequence[str]:
def _get_done_statuses(self, project: str) -> Set[str]:
Copy link
Member

Choose a reason for hiding this comment

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

should this be _get_done_states?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to match _get_done_statuses() from src/sentry/integrations/jira/.

@retry(exclude=(Integration.DoesNotExist,))
@track_group_async_operation
def sync_status_inbound(
integration_id: int, organization_id: int, issue_key: str, data: Mapping[str, Any]
Copy link
Member

Choose a reason for hiding this comment

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

I agree that providing a formless blob of data isn't ideal here, but since the task doesn't directly handle or 'expect a shape' from the data, I think it should be fine.

@mgaeta mgaeta force-pushed the feat/api-2029-webhook-task-9 branch from 7301830 to 44e9560 Compare October 2, 2021 00:34
@mgaeta mgaeta merged commit 0a9743b into master Oct 4, 2021
@mgaeta mgaeta deleted the feat/api-2029-webhook-task-9 branch October 4, 2021 15:52
@github-actions github-actions bot locked and limited conversation to collaborators Oct 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants