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

GH-33977: [Dev] PR Workflow automation bot #34161

Merged
merged 16 commits into from
Feb 28, 2023

Conversation

raulcd
Copy link
Member

@raulcd raulcd commented Feb 13, 2023

Rationale for this change

As discussed on the mailing list is quite difficult to understand the current state of a PR whether it requires further review, it has gone stale or new changes have been added. This allows us to have a set of labels based on the state of the PR.

flowchart TD
    A([New PR]):::creator
    A -- by non-committer --> B[Awaiting review]:::anyone
    A -- by committer --> C[Awaiting commiter review]:::committer
    B & C -- new review by\nanother non-committer --> C
    C & B & E  -- new committer review\nrequests changes --> D[Awaiting changes]:::creator
    D -- changes by creator --> E[Awaiting change review]:::committer
    C & E & B -- new committer review\napproves ---> F[Awaiting merge]:::committer
classDef creator stroke:#CC0;
classDef anyone stroke:#00C;
classDef committer stroke:#0C0;
classDef triager stroke:#C0C;
linkStyle 0,1,7 stroke:#CC0,color:auto;
linkStyle 2,3 stroke:#00C,color:auto;
linkStyle 4,5,6,8,9,10 stroke:#0C0,color:auto;

What changes are included in this PR?

New workflow to trigger archery bot on the required actions. New PR Workflow bot implementation on archery that manages the GitHub events and state.
New fixtures and tests.

Are these changes tested?

There are unit tests and has been tested on my fork. Some transition examples on this PR:
raulcd#74

@github-actions
Copy link

@github-actions
Copy link

⚠️ GitHub issue #33977 has been automatically assigned in GitHub to PR creator.

@raulcd raulcd marked this pull request as draft February 13, 2023 14:46
@raulcd raulcd marked this pull request as ready for review February 13, 2023 14:46
@raulcd raulcd marked this pull request as draft February 13, 2023 14:46
@raulcd raulcd marked this pull request as ready for review February 13, 2023 14:52
@raulcd raulcd marked this pull request as draft February 13, 2023 14:52
Copy link
Member

@assignUser assignUser left a comment

Choose a reason for hiding this comment

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

Thanks for taking this on! I have some comments but the main point is that we can skip using push in favor of using github.event.action and synchronize to detect added changes. This should allow some simplification in the python logic too :)

.github/workflows/pr_bot.yml Outdated Show resolved Hide resolved
.github/workflows/pr_bot.yml Show resolved Hide resolved
.github/workflows/pr_bot.yml Outdated Show resolved Hide resolved
.github/workflows/pr_bot.yml Outdated Show resolved Hide resolved
.github/workflows/pr_bot.yml Outdated Show resolved Hide resolved
.github/workflows/pr_bot.yml Outdated Show resolved Hide resolved
.github/workflows/pr_bot.yml Show resolved Hide resolved
@raulcd raulcd changed the title GH-33977: [Dev] PR Workflow automation bot WIP: GH-33977: [Dev] PR Workflow automation bot Feb 15, 2023
@raulcd raulcd changed the title WIP: GH-33977: [Dev] PR Workflow automation bot GH-33977: [Dev] PR Workflow automation bot Feb 22, 2023
@raulcd raulcd marked this pull request as ready for review February 22, 2023 12:20
@raulcd
Copy link
Member Author

raulcd commented Feb 22, 2023

I have reproduced the workflow logic on an environment like ours. I created a copy of arrow on an organization, created a fork and did some tests with a committer/non-committer user. The full workflow with the transitions can be seen on this PR: te-chie-la/arrow-pr-workflow-copy#17
The full workflow implementation uses GitHub actions to trigger archery bot tasks and some logic on archery to manage the transitions. The code on archery is tested with the different events and expected transitions.
@kou @assignUser I think this is ready for an initial round of reviews. The majority of the diff is due to a lot of fixtures with the event payloads being added for the tests.

.github/workflows/pr_bot.yml Outdated Show resolved Hide resolved
.github/workflows/pr_review_trigger.yml Outdated Show resolved Hide resolved
dev/archery/archery/bot.py Outdated Show resolved Hide resolved
dev/archery/archery/bot.py Outdated Show resolved Hide resolved
dev/archery/archery/bot.py Outdated Show resolved Hide resolved
Copy link
Member

@assignUser assignUser left a comment

Choose a reason for hiding this comment

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

One change to the workflow, overall it looks good imo. The python portions also looks fine. I don't love the if else chain to compute the current state but I also don't have anything cleaner that doesn't require a bunch of useless boiler plate :D

Very much looking forward to this getting merged!

.github/workflows/pr_bot.yml Outdated Show resolved Hide resolved
.github/workflows/pr_bot.yml Outdated Show resolved Hide resolved
.github/workflows/pr_bot.yml Outdated Show resolved Hide resolved
.github/workflows/pr_bot.yml Outdated Show resolved Hide resolved
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

Let's try this!

Copy link
Member

@assignUser assignUser left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member Author

@raulcd raulcd left a comment

Choose a reason for hiding this comment

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

+1

@raulcd raulcd merged commit 68e89ac into apache:main Feb 28, 2023
@github-actions github-actions bot added the awaiting changes Awaiting changes label Feb 28, 2023
@ursabot
Copy link

ursabot commented Mar 1, 2023

Benchmark runs are scheduled for baseline = 6cf5e89 and contender = 68e89ac. 68e89ac is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.67% ⬆️0.0%] test-mac-arm
[Finished ⬇️0.0% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.06% ⬆️0.16%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 68e89ac9 ec2-t3-xlarge-us-east-2
[Failed] 68e89ac9 test-mac-arm
[Finished] 68e89ac9 ursa-i9-9960x
[Finished] 68e89ac9 ursa-thinkcentre-m75q
[Finished] 6cf5e898 ec2-t3-xlarge-us-east-2
[Finished] 6cf5e898 test-mac-arm
[Finished] 6cf5e898 ursa-i9-9960x
[Finished] 6cf5e898 ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting changes Awaiting changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Dev] Implement automation bot for PRs
4 participants