You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "HyukjinKwon (via GitHub)" <gi...@apache.org> on 2024/02/20 23:58:27 UTC
[PR] [SPARK-47095][INFRA][FOLLOW-UP] Fix Maven test syntax errors [spark]
HyukjinKwon opened a new pull request, #45186:
URL: https://github.com/apache/spark/pull/45186
### What changes were proposed in this pull request?
This PR is a followup of https://github.com/apache/spark/pull/45171 that broke the scheduled build of macos-14.
### Why are the changes needed?
To fix up the build, It fails https://github.com/apache/spark/actions/runs/7979285164
### Does this PR introduce _any_ user-facing change?
No, test-only.
### How was this patch tested?
In my fork.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47095][INFRA][FOLLOW-UP] Remove TTY specific workaround in Maven build [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #45186: [SPARK-47095][INFRA][FOLLOW-UP] Remove TTY specific workaround in Maven build
URL: https://github.com/apache/spark/pull/45186
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47095][INFRA][FOLLOW-UP] Remove TTY specific workaround in Maven build [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #45186:
URL: https://github.com/apache/spark/pull/45186#issuecomment-1955884637
- https://github.com/HyukjinKwon/spark/actions/runs/7984135100
- https://github.com/HyukjinKwon/spark/actions/runs/7984135094
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47095][INFRA][FOLLOW-UP] Remove TTY specific workaround in Maven build [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #45186:
URL: https://github.com/apache/spark/pull/45186#discussion_r1496839930
##########
.github/workflows/maven_test.yml:
##########
@@ -178,13 +184,7 @@ jobs:
# Run the tests.
- name: Run tests
env: ${{ fromJSON(inputs.envs) }}
- # The command script takes different options ubuntu vs macos-14, see also SPARK-47095.
- shell: '[[ "${{ inputs.os }}" == *"ubuntu"* ]] && script -q -e -c "bash {0}" || script -q -e "bash {0}"'
Review Comment:
Few notes:
- The main problem here is that `inputs.os` isn't available within `shell` (see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability).
- I can't work around via environment variable either that's not replaced properly
- I can't create a custom command either because the path here should be an absolute path (home path also does not work).
- `script -q -e -c` is only workaround for ubuntu. There's no workaround for Mac (so `AmmoniteTest` fails because of TTY issue).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47095][INFRA][FOLLOW-UP] Remove TTY specific workaround in Maven build [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #45186:
URL: https://github.com/apache/spark/pull/45186#issuecomment-1955795340
Real test cases running are two:
- https://github.com/HyukjinKwon/spark/actions/runs/7983376932
- https://github.com/HyukjinKwon/spark/actions/runs/7983376928
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47095][INFRA][FOLLOW-UP] Remove TTY specific workaround in Maven build [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #45186:
URL: https://github.com/apache/spark/pull/45186#issuecomment-1955935746
Thank you again!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47095][INFRA][FOLLOW-UP] Remove TTY specific workaround in Maven build [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #45186:
URL: https://github.com/apache/spark/pull/45186#discussion_r1496823673
##########
.github/workflows/maven_test.yml:
##########
@@ -175,16 +181,9 @@ jobs:
run: |
python3.11 -m pip install 'numpy>=1.20.0' pyarrow pandas scipy unittest-xml-reporting 'grpcio==1.59.3' 'grpcio-status==1.59.3' 'protobuf==4.25.1'
python3.11 -m pip list
Review Comment:
```suggestion
python3.11 -m pip list
# Run the tests.
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47095][INFRA][FOLLOW-UP] Fix Maven test syntax errors [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #45186:
URL: https://github.com/apache/spark/pull/45186#issuecomment-1955475228
Please ping me when the PR is ready.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47095][INFRA][FOLLOW-UP] Remove TTY specific workaround in Maven build [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #45186:
URL: https://github.com/apache/spark/pull/45186#discussion_r1496823673
##########
.github/workflows/maven_test.yml:
##########
@@ -175,16 +181,9 @@ jobs:
run: |
python3.11 -m pip install 'numpy>=1.20.0' pyarrow pandas scipy unittest-xml-reporting 'grpcio==1.59.3' 'grpcio-status==1.59.3' 'protobuf==4.25.1'
python3.11 -m pip list
Review Comment:
```suggestion
python3.11 -m pip list
# Run the tests.
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47095][INFRA][FOLLOW-UP] Remove TTY specific workaround in Maven build [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #45186:
URL: https://github.com/apache/spark/pull/45186#discussion_r1496839930
##########
.github/workflows/maven_test.yml:
##########
@@ -178,13 +184,7 @@ jobs:
# Run the tests.
- name: Run tests
env: ${{ fromJSON(inputs.envs) }}
- # The command script takes different options ubuntu vs macos-14, see also SPARK-47095.
- shell: '[[ "${{ inputs.os }}" == *"ubuntu"* ]] && script -q -e -c "bash {0}" || script -q -e "bash {0}"'
Review Comment:
Few notes:
- The main problem here is that `inputs.os` isn't available within `shell` (see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability).
- I can't work around via environment variable either that's not replaced properly. e.g., `$HOME` is used as a plain string `$HOME`.
- I can't create a custom command either because the path here should be an absolute path (home path also does not work).
- `script -q -e -c` is only workaround for ubuntu. There's no workaround for Mac (so `AmmoniteTest` fails because of TTY issue).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47095][INFRA][FOLLOW-UP] Remove TTY specific workaround in Maven build [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #45186:
URL: https://github.com/apache/spark/pull/45186#discussion_r1496837445
##########
.github/workflows/maven_test.yml:
##########
@@ -73,13 +73,19 @@ jobs:
connector#kafka-0-10,connector#kafka-0-10-sql,connector#kafka-0-10-token-provider,connector#spark-ganglia-lgpl,connector#protobuf,connector#avro
- >-
sql#api,sql#catalyst,resource-managers#yarn,resource-managers#kubernetes#core
- - >-
- connect
# Here, we split Hive and SQL tests into some of slow ones and the rest of them.
included-tags: [ "" ]
excluded-tags: [ "" ]
comment: [ "" ]
include:
+ # Connect tests
+ - modules: connect
+ java: ${{ inputs.java }}
+ hadoop: ${{ inputs.hadoop }}
+ hive: hive2.3
+ # TODO(SPARK-47110): Reenble AmmoniteTest tests in Maven builds
+ excluded-tags: org.apache.spark.tags.AmmoniteTest
Review Comment:
Ya, this was a known issue. I disabled this previously.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47095][INFRA][FOLLOW-UP] Remove TTY specific workaround in Maven build [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #45186:
URL: https://github.com/apache/spark/pull/45186#issuecomment-1955935033
The links show successes. Let's merge this.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org