Skip to content

Trigger test tasks selected by bugbug and publish test results on Phabricator#264

Merged
marco-c merged 4 commits intomozilla:masterfrom
marco-c:trigger_tests
Nov 29, 2019
Merged

Trigger test tasks selected by bugbug and publish test results on Phabricator#264
marco-c merged 4 commits intomozilla:masterfrom
marco-c:trigger_tests

Conversation

@marco-c
Copy link
Copy Markdown
Collaborator

@marco-c marco-c commented Nov 25, 2019

No description provided.

Copy link
Copy Markdown
Collaborator

@La0 La0 left a comment

Choose a reason for hiding this comment

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

I'm not happy with the current situation where we put more & more stuff in code-review events, at the risk of breaking the relative stability we have acquired.

One solution i would like to try, is to put the code in there, but not execute everything on the same instance.
We now have multiple dynos running in production: 1 web dyno + N worker dyno. That works because the web dyno is the only one queueing in the build queue. But adding pulse listeners on every workers would be bad.

So we should add feature flags to activate web+pulse queuing on the web dyno and heavy lifting on worker dynos. On dev you could have everything running at once.

Comment thread events/code_review_events/workflow.py Outdated
Comment thread events/code_review_events/workflow.py Outdated
Comment thread events/code_review_events/workflow.py Outdated
Comment thread events/code_review_events/workflow.py Outdated
Comment thread events/code_review_events/workflow.py Outdated
Comment thread events/code_review_events/workflow.py Outdated
Comment thread events/code_review_events/workflow.py Outdated
Comment thread events/code_review_events/workflow.py Outdated
Comment thread events/code_review_events/workflow.py Outdated
# self.workflow.run(),
# Add mercurial task
self.mercurial.run(),
# self.mercurial.run(),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's not forget to remove those

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Why? This was the one change that reduced the risk of new bugs to 0!

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🥁

Comment thread events/code_review_events/workflow.py Outdated
Copy link
Copy Markdown
Collaborator

@La0 La0 left a comment

Choose a reason for hiding this comment

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

Why don't we have a separate instance of events for this stuff already ?

Comment thread events/code_review_events/workflow.py Outdated
@marco-c
Copy link
Copy Markdown
Collaborator Author

marco-c commented Nov 27, 2019

As discussed on IRC, a good middle ground is having it as a separate async task communicating with the main workflow using queues.

@marco-c marco-c force-pushed the trigger_tests branch 5 times, most recently from 63be68e to 60ec7f8 Compare November 28, 2019 17:49
@marco-c marco-c requested a review from La0 November 28, 2019 17:53
Comment thread events/code_review_events/workflow.py Outdated
@marco-c marco-c changed the title [WIP] Trigger test tasks selected by bugbug and publish test results on Phabricator Trigger test tasks selected by bugbug and publish test results on Phabricator Nov 29, 2019
Copy link
Copy Markdown
Collaborator

@La0 La0 left a comment

Choose a reason for hiding this comment

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

Let's 🚢 the damn thing !

@marco-c
Copy link
Copy Markdown
Collaborator Author

marco-c commented Nov 29, 2019

Let's ship the damn thing !

\o/

@marco-c marco-c merged commit 17f5562 into mozilla:master Nov 29, 2019
@marco-c marco-c deleted the trigger_tests branch November 29, 2019 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants