You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "adoroszlai (via GitHub)" <gi...@apache.org> on 2024/11/27 14:38:00 UTC
[PR] HDDS-11813. Reduce duplication in CI workflow [ozone]
adoroszlai opened a new pull request, #7497:
URL: https://github.com/apache/ozone/pull/7497
## What changes were proposed in this pull request?
- Create a reusable workflow (`check.yml`) for a single, generic check.
- Refactor most existing checks in `ci.yml` (except `build-info`, which has completely different logic, and `coverage`, which needs extra tokens) to call the reusable one.
- Move some check-specific logic into the check scripts.
- Collect output.log for `build` check.
- Create separate check script for `compile` for simpler use.
- Add `test-` prefix for `integration` test profiles. (This is needed for checking test profile in `integration.sh`.)
https://issues.apache.org/jira/browse/HDDS-11813
## How was this patch tested?
CI:
https://github.com/adoroszlai/ozone/actions/runs/12049965206
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11813. Reduce duplication in CI workflow [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7497:
URL: https://github.com/apache/ozone/pull/7497#issuecomment-2618036573
I'll extract parts of this to separate PRs to make it easier to review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11813. Reduce duplication in CI workflow [ozone]
Posted by "sadanand48 (via GitHub)" <gi...@apache.org>.
sadanand48 commented on code in PR #7497:
URL: https://github.com/apache/ozone/pull/7497#discussion_r1882476065
##########
.github/workflows/ci.yml:
##########
@@ -27,15 +27,14 @@ on:
default: ''
required: false
env:
- FAIL_FAST: ${{ github.event_name == 'pull_request' }}
+ BUILD_ARGS: "-Pdist -Psrc -Dmaven.javadoc.skip=true"
# Minimum required Java version for running Ozone is defined in pom.xml (javac.version).
TEST_JAVA_VERSION: 21 # JDK version used by CI build and tests; should match the JDK version in apache/ozone-runner image
+ # MAVEN_ARGS and MAVEN_OPTS are duplicated in check.yml, please keep in sync
Review Comment:
why is this done ? can't we pass these to check.yml via env
##########
hadoop-ozone/dev-support/checks/build.sh:
##########
@@ -14,7 +14,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-set -eu -o pipefail
+set -u -o pipefail
Review Comment:
why did we remove -e here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11813. Reduce duplication in CI workflow [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7497:
URL: https://github.com/apache/ozone/pull/7497#issuecomment-2710732050
Thanks @fapifta, @sadanand48 for the review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11813. Reduce duplication in CI workflow [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #7497:
URL: https://github.com/apache/ozone/pull/7497#discussion_r1882550888
##########
.github/workflows/ci.yml:
##########
@@ -27,15 +27,14 @@ on:
default: ''
required: false
env:
- FAIL_FAST: ${{ github.event_name == 'pull_request' }}
+ BUILD_ARGS: "-Pdist -Psrc -Dmaven.javadoc.skip=true"
# Minimum required Java version for running Ozone is defined in pom.xml (javac.version).
TEST_JAVA_VERSION: 21 # JDK version used by CI build and tests; should match the JDK version in apache/ozone-runner image
+ # MAVEN_ARGS and MAVEN_OPTS are duplicated in check.yml, please keep in sync
Review Comment:
> can't we pass these to check.yml via env
Unfortunately not.
We can pass inputs to `check.yml` via `with`, like:
https://github.com/apache/ozone/blob/303e3bf4bfc66ae3aa5cab50c9b73dee88b0ef6d/.github/workflows/ci.yml#L113-L116
However, we cannot use `${{ env.MAVEN_ARGS }}` in item of `with`. Only `github` and `needs` contexts are available.
https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#jobsjob_idwithinput_id
Some of the other environment variables we pass by adding as `output` of `build-info` (so via `needs`):
https://github.com/apache/ozone/blob/303e3bf4bfc66ae3aa5cab50c9b73dee88b0ef6d/.github/workflows/ci.yml#L55-L58
https://github.com/apache/ozone/blob/303e3bf4bfc66ae3aa5cab50c9b73dee88b0ef6d/.github/workflows/ci.yml#L119
- `OZONE_WITH_COVERAGE` has logic based on the event that triggers `ci.yml`, we cannot access that in `check.yml`.
- `build-args` and `java-version` are passed to inputs that exist anyway.
I didn't want to add new inputs for these `MAVEN_` constants. Duplication seemed to be the better choice in this case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11813. Reduce duplication in CI workflow [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #7497:
URL: https://github.com/apache/ozone/pull/7497#discussion_r1935668568
##########
hadoop-ozone/dev-support/checks/build.sh:
##########
@@ -14,7 +14,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-set -eu -o pipefail
+set -u -o pipefail
Review Comment:
Extracted the part related to saving logs for build/compile checks to #7782. The only difference from here is addition of `upload-artifact` in `ci.yml`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11813. Reduce duplication in CI workflow [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #7497:
URL: https://github.com/apache/ozone/pull/7497
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11813. Reduce duplication in CI workflow [ozone]
Posted by "sadanand48 (via GitHub)" <gi...@apache.org>.
sadanand48 commented on code in PR #7497:
URL: https://github.com/apache/ozone/pull/7497#discussion_r1883562572
##########
hadoop-ozone/dev-support/checks/build.sh:
##########
@@ -14,7 +14,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-set -eu -o pipefail
+set -u -o pipefail
Review Comment:
Thanks for explaining.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11813. Reduce duplication in CI workflow [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #7497:
URL: https://github.com/apache/ozone/pull/7497#discussion_r1882559334
##########
hadoop-ozone/dev-support/checks/build.sh:
##########
@@ -14,7 +14,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-set -eu -o pipefail
+set -u -o pipefail
Review Comment:
`-e` causes the script to exit as soon as it encounters an error, e.g. if the `mvn` command fails. Post-processing logic (search for errors in the log, etc.) requires the script to continue. It exits with the proper value in the end.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org