feat(gh-comments): Queuing Logic #50865
Conversation
This change allows the gh comment task to be queued from post process.
github-actions
bot
added
the
Scope: Backend
| Mock( | ||
| return_value={ | ||
| "commitId": "asdfwreqr", | ||
| "committedDate": "2023-02-14T11:11Z", |
There was a problem hiding this comment.
Isn't it better to add a date() here so you make sure in tests the commit is always recent?
There was a problem hiding this comment.
the commit date isn't important in this case. I'd rather just mock a response once here and have the object be fixed.
src/sentry/tasks/commit_context.py
Outdated
| ) | ||
| if ( | ||
| pr.exists() | ||
| and pr[0].date_added >= datetime.now(tz=timezone.utc) - timedelta(days=30) |
There was a problem hiding this comment.
Would pr ever include more than one object? and if so why do we choose pr[0] over the others?
There was a problem hiding this comment.
pr is technically a query so we might want to call it pr_query. we could also use get() but get() throws an error if the item doesn't exist or if multiple items exist that match the criteria
There was a problem hiding this comment.
although we should only have one match so we could use get()
src/sentry/tasks/commit_context.py
Outdated
| ) | ||
| if ( | ||
| pr.exists() | ||
| and pr[0].date_added >= datetime.now(tz=timezone.utc) - timedelta(days=30) |
There was a problem hiding this comment.
although we should only have one match so we could use get()
| ), | ||
| ) | ||
| @patch("sentry.tasks.integrations.github.pr_comment.comment_workflow.delay") | ||
| class TestGHCommentQueuing(TestCommitContextMixin): |
There was a problem hiding this comment.
does putting all the patches before the class mean that all the patches are applied to the tests inside the class?
There was a problem hiding this comment.
yup! Saves on code repetition. I think you can override it too if you have one or two tests that want a different patch
Add task queuing logic to post process, specifically in process_commit_context when a suspect committer is established.