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

[ews-build.webkit.org] Make GitHubMixin.get_pr_json asynchronous #11954

Conversation

JonWBedard
Copy link
Member

@JonWBedard JonWBedard commented Mar 24, 2023

9a18fa3

[ews-build.webkit.org] Make GitHubMixin.get_pr_json asynchronous
https://bugs.webkit.org/show_bug.cgi?id=254446
rdar://107207345

Reviewed by Aakash Jain.

* Tools/CISupport/ews-build/steps.py:
(GitHubMixin):
(GitHubMixin.get_pr_json): Use Twisted's asynchronous request library
and make function asynchronous.
(GitHubMixin.should_send_email_for_pr): get_pr_json is not asynchronous.
(ValidateChange.validate_github): Ditto.
(BlockPullRequest.run): Ditto.
(GitHubMixin.fetch_data_from_url_with_authentication_github_old): Deleted.

Canonical link: https://commits.webkit.org/262151@main

7e8886c

Misc iOS, tvOS & watchOS macOS Linux Windows
  πŸ§ͺ style   πŸ›  ios   πŸ›  mac   πŸ›  wpe   πŸ›  wincairo
  πŸ§ͺ bindings   πŸ›  ios-sim   πŸ›  mac-AS-debug   πŸ§ͺ wpe-wk2
  πŸ§ͺ webkitperl   πŸ§ͺ ios-wk2   πŸ§ͺ api-mac   πŸ›  gtk
  πŸ§ͺ api-ios   πŸ§ͺ mac-wk1   πŸ§ͺ gtk-wk2
  πŸ›  tv   πŸ§ͺ mac-wk2   πŸ§ͺ api-gtk
  πŸ›  tv-sim   πŸ§ͺ mac-AS-debug-wk2
  πŸ§ͺ services   πŸ›  watch   πŸ§ͺ mac-wk2-stress
  πŸ›  watch-sim
βœ… πŸ›  πŸ§ͺ unsafe-merge

@JonWBedard JonWBedard self-assigned this Mar 24, 2023
@JonWBedard JonWBedard added the Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases label Mar 24, 2023
Copy link
Member

@aj062 aj062 left a comment

Choose a reason for hiding this comment

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

r+ assuming this is tested

Copy link
Member

@aj062 aj062 left a comment

Choose a reason for hiding this comment

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

Slightly unrelated. I see some _addToLog calls which doesn't have yield in GitHubMixin.get_reviewers() and GitHubMixin.should_send_email_for_pr()

@JonWBedard
Copy link
Member Author

r+ assuming this is tested

This one has been live on the UAT instance all weekend: https://ews-build.webkit-uat.org/#/builders/4/builds/7580

@JonWBedard
Copy link
Member Author

Slightly unrelated. I see some _addToLog calls which doesn't have yield in GitHubMixin.get_reviewers() and GitHubMixin.should_send_email_for_pr()

Probably more related to #11975 and #11958 respectively, will make sure those PRs have the relevant fixes.

@JonWBedard JonWBedard added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing labels Mar 27, 2023
@JonWBedard
Copy link
Member Author

Slightly unrelated. I see some _addToLog calls which doesn't have yield in GitHubMixin.get_reviewers() and GitHubMixin.should_send_email_for_pr()

Probably more related to #11975 and #11958 respectively, will make sure those PRs have the relevant fixes.

Actually, scratch that, I see why you bring that up in this PR. You're right, this is the more appropriate place to fix this deficiency.

@JonWBedard JonWBedard force-pushed the eng/ews-build-webkit-org-Make-GitHubMixin-get_pr_json-asynchronous branch from 2933745 to 7e8886c Compare March 27, 2023 15:19
@JonWBedard JonWBedard added skip-ews Applied to prevent a change from being run on EWS unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing labels Mar 27, 2023
https://bugs.webkit.org/show_bug.cgi?id=254446
rdar://107207345

Reviewed by Aakash Jain.

* Tools/CISupport/ews-build/steps.py:
(GitHubMixin):
(GitHubMixin.get_pr_json): Use Twisted's asynchronous request library
and make function asynchronous.
(GitHubMixin.should_send_email_for_pr): get_pr_json is not asynchronous.
(ValidateChange.validate_github): Ditto.
(BlockPullRequest.run): Ditto.
(GitHubMixin.fetch_data_from_url_with_authentication_github_old): Deleted.

Canonical link: https://commits.webkit.org/262151@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/ews-build-webkit-org-Make-GitHubMixin-get_pr_json-asynchronous branch from 7e8886c to 9a18fa3 Compare March 27, 2023 15:24
@webkit-commit-queue
Copy link
Collaborator

Committed 262151@main (9a18fa3): https://commits.webkit.org/262151@main

Reviewed commits have been landed. Closing PR #11954 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Mar 27, 2023
@webkit-commit-queue webkit-commit-queue merged commit 9a18fa3 into WebKit:main Mar 27, 2023
@JonWBedard JonWBedard deleted the eng/ews-build-webkit-org-Make-GitHubMixin-get_pr_json-asynchronous branch April 18, 2023 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-ews Applied to prevent a change from being run on EWS Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases
Projects
None yet
4 participants