-
Notifications
You must be signed in to change notification settings - Fork 387
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
[#768] improvement(CI): Avoiding related issues by changing CI pipeline dependencies #1500
Conversation
|
||
- name: Package Gravitino | ||
run: | | ||
./gradlew build -x test -PjdkVersion=${{ matrix.java-version }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this step still required? As we've already build the project above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the job name is 'Package Gravitino' and it is indeed a packaging task, the build
task can be skipped and use compileDistribution
directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried. But It will fail. I recover the command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I execute ./gradlew compileDistribution -x test
locally without executing ./gradlew build
, and it seems no error exists
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't work. You can see https://github.com/datastrato/gravitino/actions/runs/7536000721
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wander why ./gradlew compileDistribution -x test
will trigger integration test...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wander why
./gradlew compileDistribution -x test
will trigger integration test...
We have next steps. The step which you mentioned doesn't trigger.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have already built the Gravitino in line 85, I was thinking why can't we reuse the built package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They're different jobs. Different jobs may be scheduled to different machines.
I recommend changing the title to "Avoiding Related Issues by Changing CI Pipeline Dependencies" or something similar. |
done. |
One thing I was thinking is what is the benefit of this change? I can see the total CI time will be increased because we make it in sequence, with the cost of it what was the actual benefit? |
We can save the resource of Github action. That's a good point. But for CI time, this is not a good choice. |
What changes were proposed in this pull request?
Setting the job dependencies. There are some issues if we use two workflows. I change them to one workflow.
Why are the changes needed?
Fix: #768
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Tested in my repo.
qqqttt123#4