Skip to content

/ servo

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

Invert worker type selection logic for taskcluster. #23106

Merged
merged 1 commit into from Mar 27, 2019
Merged

Invert worker type selection logic for taskcluster. #23106

merged 1 commit into from Mar 27, 2019

Conversation

@jdm
Copy link
Member

jdm commented Mar 26, 2019 •
edited by SimonSapin

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #23105
  • There are tests for these changes

This change is Reviewable
@jdm
Copy link
Member Author

jdm commented Mar 26, 2019

I'm going to see what the CI results for this are before trying to merge it.
@jdm
Copy link
Member Author

jdm commented Mar 26, 2019

@bors-servo r=SimonSapin
@bors-servo
Copy link
Contributor

bors-servo commented Mar 26, 2019

📌 Commit 04669f9 has been approved by SimonSapin
@highfive highfive assigned SimonSapin and unassigned emilio Mar 26, 2019
@SimonSapin
Copy link
Member

SimonSapin commented Mar 26, 2019

Sorry, I think I misspoke on IRC. Literally inverting the logic by reversing the order of the or’s operands doesn’t seem ideal : self.worker_type is never falsy, so this line is effectively .with_worker_type(self.worker_type).

Maybe instead we should set CONFIG.docker_image_buil_worker_type to None for untrusted tasks (and fix the typo). That way, whenever we start testing on Android emulators on Packet.net again, we can still have that config make docker images build on EC2, leaving the more scarce worker(s) for tasks that actually need it.
@jdm
Copy link
Member Author

jdm commented Mar 26, 2019

@bors-servo r-
@jdm
Use provided docker builder worker for github PR tasks.
Loading status checks…
03f79b2
@jdm jdm force-pushed the jdm-patch-30 branch from 04669f9 to 03f79b2 Mar 26, 2019
@jdm
Copy link
Member Author

jdm commented Mar 26, 2019

That change does seem like an improvement over the previous attempt.
@SimonSapin
Copy link
Member

SimonSapin commented Mar 27, 2019

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Mar 27, 2019

📌 Commit 03f79b2 has been approved by SimonSapin
@bors-servo
Copy link
Contributor

bors-servo commented Mar 27, 2019

⌛ Testing commit 03f79b2 with merge c07f477...
bors-servo added a commit that referenced this pull request Mar 27, 2019
@bors-servo
Auto merge of #23106 - servo:jdm-patch-30, r=SimonSapin
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
c07f477 
Invert worker type selection logic for taskcluster.

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #23105
- [x] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23106)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Mar 27, 2019

💔 Test failed - mac-rel-wpt2
@CYBAI
Copy link
Collaborator

CYBAI commented Mar 27, 2019

@bors-servo retry
@bors-servo
Copy link
Contributor

bors-servo commented Mar 27, 2019

⌛ Testing commit 03f79b2 with merge 0cb87cc...
bors-servo added a commit that referenced this pull request Mar 27, 2019
@bors-servo
Auto merge of #23106 - servo:jdm-patch-30, r=SimonSapin
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
0cb87cc 
Invert worker type selection logic for taskcluster.

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #23105
- [x] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23106)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Mar 27, 2019

☀️ Test successful - android-mac, arm32, arm64, linux-rel-css, linux-rel-wpt, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, magicleap, status-taskcluster
Approved by: SimonSapin
Pushing 0cb87cc to master...
@bors-servo bors-servo merged commit 03f79b2 into master Mar 27, 2019
4 checks passed
4 checks passed
Taskcluster (pull_request) TaskGroup: success
Details
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
@SimonSapin SimonSapin deleted the jdm-patch-30 branch Mar 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@SimonSapin SimonSapin

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

Untrusted decision tasks are erroring out
6 participants
@jdm @bors-servo @SimonSapin @CYBAI @emilio @highfive
You can’t perform that action at this time.