You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "mohamedawnallah (via GitHub)" <gi...@apache.org> on 2025/07/15 17:54:54 UTC
[PR] workflows: update marker from `require_non_self_hosted` to `require_docker_in_docker` in `Pposargs` [beam]
mohamedawnallah opened a new pull request, #35598:
URL: https://github.com/apache/beam/pull/35598
## Description
Follow-up on https://github.com/apache/beam/pull/35585
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://github.com/apache/beam/blob/master/CONTRIBUTING.md#make-the-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Go+tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI or the [workflows README](https://github.com/apache/beam/blob/master/.github/workflows/README.md) to see a list of phrases to trigger workflows.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] workflows: update marker from `require_non_self_hosted` to `require_docker_in_docker` in `Pposargs` [beam]
Posted by "mohamedawnallah (via GitHub)" <gi...@apache.org>.
mohamedawnallah commented on code in PR #35598:
URL: https://github.com/apache/beam/pull/35598#discussion_r2208235235
##########
.github/workflows/beam_PostCommit_Python.yml:
##########
@@ -107,8 +107,8 @@ jobs:
-PuseWheelDistribution \
-Pposargs="${{
contains(matrix.os, 'self-hosted') &&
- '-m ''not require_non_self_hosted''' ||
- '-m ''require_non_self_hosted'''
+ '-m ''not require_docker_in_docker''' ||
+ '-m ''require_docker_in_docker'''
}}" \
Review Comment:
I am not sure if the updated `Pposargs` are maintainable and extendable in the future incase if there are other environmental constraints in the future. I feel like `require_non_self_hosted` would be more extendable...
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] workflows: update marker from `require_non_self_hosted` to `require_docker_in_docker` in `Pposargs` [beam]
Posted by "mohamedawnallah (via GitHub)" <gi...@apache.org>.
mohamedawnallah commented on code in PR #35598:
URL: https://github.com/apache/beam/pull/35598#discussion_r2208479130
##########
.github/workflows/beam_PreCommit_Python_ML.yml:
##########
@@ -104,8 +104,8 @@ jobs:
arguments: |
-Pposargs="${{
contains(matrix.os, 'self-hosted') &&
- 'apache_beam/ml/ -m ''not require_non_self_hosted''' ||
- 'apache_beam/ml/ -m ''require_non_self_hosted'''
+ 'apache_beam/ml/ -m ''not require_docker_in_docker''' ||
+ 'apache_beam/ml/ -m ''require_docker_in_docker'''
Review Comment:
> Probably a simple fix would be to replace
>
> https://github.com/apache/beam/blob/8930c8dd5f8ef97cdceddbec792aec08455f56f7/sdks/python/scripts/run_pytest.sh#L34
>
> and instead of failing, collect the extra `-m` arguments and add the appropriate `not no_xdist` and `no_xdist` flags in
>
> https://github.com/apache/beam/blob/8930c8dd5f8ef97cdceddbec792aec08455f56f7/sdks/python/scripts/run_pytest.sh#L44-L48
This approach looks good 👍
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] workflows: update marker from `require_non_self_hosted` to `require_docker_in_docker` in `Pposargs` [beam]
Posted by "mohamedawnallah (via GitHub)" <gi...@apache.org>.
mohamedawnallah commented on code in PR #35598:
URL: https://github.com/apache/beam/pull/35598#discussion_r2208235235
##########
.github/workflows/beam_PostCommit_Python.yml:
##########
@@ -107,8 +107,8 @@ jobs:
-PuseWheelDistribution \
-Pposargs="${{
contains(matrix.os, 'self-hosted') &&
- '-m ''not require_non_self_hosted''' ||
- '-m ''require_non_self_hosted'''
+ '-m ''not require_docker_in_docker''' ||
+ '-m ''require_docker_in_docker'''
}}" \
Review Comment:
I am not sure if the updated `Pposargs` are maintainable and extendable in the future incase if there are other environmental constraints in the future. I feel like `require_non_self_hosted` marker would be more extendable...
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] workflows: update marker from `require_non_self_hosted` to `require_docker_in_docker` in `Pposargs` [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #35598:
URL: https://github.com/apache/beam/pull/35598#discussion_r2208322353
##########
.github/workflows/beam_PreCommit_Python_ML.yml:
##########
@@ -104,8 +104,8 @@ jobs:
arguments: |
-Pposargs="${{
contains(matrix.os, 'self-hosted') &&
- 'apache_beam/ml/ -m ''not require_non_self_hosted''' ||
- 'apache_beam/ml/ -m ''require_non_self_hosted'''
+ 'apache_beam/ml/ -m ''not require_docker_in_docker''' ||
+ 'apache_beam/ml/ -m ''require_docker_in_docker'''
Review Comment:
https://github.com/apache/beam/actions/runs/16300739721/job/46034393155
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] workflows: update marker from `require_non_self_hosted` to `require_docker_in_docker` in `Pposargs` [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm merged PR #35598:
URL: https://github.com/apache/beam/pull/35598
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] workflows: update marker from `require_non_self_hosted` to `require_docker_in_docker` in `Pposargs` [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #35598:
URL: https://github.com/apache/beam/pull/35598#discussion_r2208322100
##########
.github/workflows/beam_PreCommit_Python_ML.yml:
##########
@@ -104,8 +104,8 @@ jobs:
arguments: |
-Pposargs="${{
contains(matrix.os, 'self-hosted') &&
- 'apache_beam/ml/ -m ''not require_non_self_hosted''' ||
- 'apache_beam/ml/ -m ''require_non_self_hosted'''
+ 'apache_beam/ml/ -m ''not require_docker_in_docker''' ||
+ 'apache_beam/ml/ -m ''require_docker_in_docker'''
Review Comment:
Hm, it looks like this will currently not work because of https://github.com/apache/beam/blob/8930c8dd5f8ef97cdceddbec792aec08455f56f7/sdks/python/scripts/run_pytest.sh#L34
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] workflows: update marker from `require_non_self_hosted` to `require_docker_in_docker` in `Pposargs` [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #35598:
URL: https://github.com/apache/beam/pull/35598#discussion_r2208337172
##########
.github/workflows/beam_PreCommit_Python_ML.yml:
##########
@@ -104,8 +104,8 @@ jobs:
arguments: |
-Pposargs="${{
contains(matrix.os, 'self-hosted') &&
- 'apache_beam/ml/ -m ''not require_non_self_hosted''' ||
- 'apache_beam/ml/ -m ''require_non_self_hosted'''
+ 'apache_beam/ml/ -m ''not require_docker_in_docker''' ||
+ 'apache_beam/ml/ -m ''require_docker_in_docker'''
Review Comment:
Probably a simple fix would be to replace https://github.com/apache/beam/blob/8930c8dd5f8ef97cdceddbec792aec08455f56f7/sdks/python/scripts/run_pytest.sh#L34
and instead of failing, collect the extra `-m` arguments and add the appropriate `not no_xdist` and `no_xdist` flags in
https://github.com/apache/beam/blob/8930c8dd5f8ef97cdceddbec792aec08455f56f7/sdks/python/scripts/run_pytest.sh#L44-L48
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] workflows: update marker from `require_non_self_hosted` to `require_docker_in_docker` in `Pposargs` [beam]
Posted by "mohamedawnallah (via GitHub)" <gi...@apache.org>.
mohamedawnallah commented on code in PR #35598:
URL: https://github.com/apache/beam/pull/35598#discussion_r2208478134
##########
.github/workflows/beam_PreCommit_Python_ML.yml:
##########
@@ -104,8 +104,8 @@ jobs:
arguments: |
-Pposargs="${{
contains(matrix.os, 'self-hosted') &&
- 'apache_beam/ml/ -m ''not require_non_self_hosted''' ||
- 'apache_beam/ml/ -m ''require_non_self_hosted'''
+ 'apache_beam/ml/ -m ''not require_docker_in_docker''' ||
+ 'apache_beam/ml/ -m ''require_docker_in_docker'''
Review Comment:
Just seen this notification comments...
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] workflows: update marker from `require_non_self_hosted` to `require_docker_in_docker` in `Pposargs` [beam]
Posted by "mohamedawnallah (via GitHub)" <gi...@apache.org>.
mohamedawnallah commented on code in PR #35598:
URL: https://github.com/apache/beam/pull/35598#discussion_r2208235235
##########
.github/workflows/beam_PostCommit_Python.yml:
##########
@@ -107,8 +107,8 @@ jobs:
-PuseWheelDistribution \
-Pposargs="${{
contains(matrix.os, 'self-hosted') &&
- '-m ''not require_non_self_hosted''' ||
- '-m ''require_non_self_hosted'''
+ '-m ''not require_docker_in_docker''' ||
+ '-m ''require_docker_in_docker'''
}}" \
Review Comment:
I am not sure if the updated `Pposargs` are maintainable and extendable in the future incase if there are other environmental constrains in the future. I feel like `require_non_self_hosted` would be more extendable...
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] workflows: update marker from `require_non_self_hosted` to `require_docker_in_docker` in `Pposargs` [beam]
Posted by "mohamedawnallah (via GitHub)" <gi...@apache.org>.
mohamedawnallah commented on code in PR #35598:
URL: https://github.com/apache/beam/pull/35598#discussion_r2208235235
##########
.github/workflows/beam_PostCommit_Python.yml:
##########
@@ -107,8 +107,8 @@ jobs:
-PuseWheelDistribution \
-Pposargs="${{
contains(matrix.os, 'self-hosted') &&
- '-m ''not require_non_self_hosted''' ||
- '-m ''require_non_self_hosted'''
+ '-m ''not require_docker_in_docker''' ||
+ '-m ''require_docker_in_docker'''
}}" \
Review Comment:
I am not sure if the updated `Pposargs` are maintainable and extendable in the future incase if there are other environmental constrains in the future. I feel like `require_non_self_hosted` would be more extendable
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] workflows: update marker from `require_non_self_hosted` to `require_docker_in_docker` in `Pposargs` [beam]
Posted by "mohamedawnallah (via GitHub)" <gi...@apache.org>.
mohamedawnallah commented on PR #35598:
URL: https://github.com/apache/beam/pull/35598#issuecomment-3074723686
cc @damccorm
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] workflows: update marker from `require_non_self_hosted` to `require_docker_in_docker` in `Pposargs` [beam]
Posted by "mohamedawnallah (via GitHub)" <gi...@apache.org>.
mohamedawnallah commented on code in PR #35598:
URL: https://github.com/apache/beam/pull/35598#discussion_r2208480910
##########
.github/workflows/beam_PreCommit_Python_ML.yml:
##########
@@ -104,8 +104,8 @@ jobs:
arguments: |
-Pposargs="${{
contains(matrix.os, 'self-hosted') &&
- 'apache_beam/ml/ -m ''not require_non_self_hosted''' ||
- 'apache_beam/ml/ -m ''require_non_self_hosted'''
+ 'apache_beam/ml/ -m ''not require_docker_in_docker''' ||
+ 'apache_beam/ml/ -m ''require_docker_in_docker'''
Review Comment:
I've also another approach of just using `-k` instead `-m` for keyword matching but I think your approach is more reliable in the future
```bash
(.venv) dev@dev:~/beam_dev/beam/sdks/python/apache_beam/ml/rag/enrichment$ pytest -v -k "require_docker_in_docker" --test-pipeline-options="--runner=DirectRunner"
====================================================== test session starts =======================================================
platform linux -- Python 3.9.22, pytest-7.4.4, pluggy-1.6.0 -- /home/dev/beam_dev/beam/sdks/python/.venv/bin/python
cachedir: .pytest_cache
hypothesis profile 'default'
rootdir: /home/dev/beam_dev/beam/sdks/python
configfile: pytest.ini
plugins: xdist-3.8.0, requests-mock-1.12.1, timeout-2.4.0, anyio-4.9.0, hypothesis-6.135.31
timeout: 600.0s
timeout method: signal
timeout func_only: False
collected 36 items / 27 deselected / 9 selected
milvus_search_it_test.py::TestMilvusSearchEnrichment::test_empty_input_chunks PASSED [ 11%]
milvus_search_it_test.py::TestMilvusSearchEnrichment::test_filtered_search_with_bm25_full_text_and_batching PASSED [ 22%]
milvus_search_it_test.py::TestMilvusSearchEnrichment::test_filtered_search_with_cosine_similarity_and_batching PASSED [ 33%]
milvus_search_it_test.py::TestMilvusSearchEnrichment::test_hybrid_search PASSED [ 44%]
milvus_search_it_test.py::TestMilvusSearchEnrichment::test_invalid_query_on_non_existent_collection PASSED [ 55%]
milvus_search_it_test.py::TestMilvusSearchEnrichment::test_invalid_query_on_non_existent_field PASSED [ 66%]
milvus_search_it_test.py::TestMilvusSearchEnrichment::test_keyword_search_with_inner_product_sparse_embedding PASSED [ 77%]
milvus_search_it_test.py::TestMilvusSearchEnrichment::test_vector_search_with_euclidean_distance PASSED [ 88%]
milvus_search_it_test.py::TestMilvusSearchEnrichment::test_vector_search_with_inner_product_similarity PASSED [100%]
=============================================== 9 passed, 27 deselected in 30.26s ================================================
```
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] workflows: update marker from `require_non_self_hosted` to `require_docker_in_docker` in `Pposargs` [beam]
Posted by "mohamedawnallah (via GitHub)" <gi...@apache.org>.
mohamedawnallah commented on code in PR #35598:
URL: https://github.com/apache/beam/pull/35598#discussion_r2208480910
##########
.github/workflows/beam_PreCommit_Python_ML.yml:
##########
@@ -104,8 +104,8 @@ jobs:
arguments: |
-Pposargs="${{
contains(matrix.os, 'self-hosted') &&
- 'apache_beam/ml/ -m ''not require_non_self_hosted''' ||
- 'apache_beam/ml/ -m ''require_non_self_hosted'''
+ 'apache_beam/ml/ -m ''not require_docker_in_docker''' ||
+ 'apache_beam/ml/ -m ''require_docker_in_docker'''
Review Comment:
I've also another approach of just using `-k` instead `-m` for keyword matching:
```bash
(.venv) dev@dev:~/beam_dev/beam/sdks/python/apache_beam/ml/rag/enrichment$ pytest -v -k "require_docker_in_docker" --test-pipeline-options="--runner=DirectRunner"
====================================================== test session starts =======================================================
platform linux -- Python 3.9.22, pytest-7.4.4, pluggy-1.6.0 -- /home/dev/beam_dev/beam/sdks/python/.venv/bin/python
cachedir: .pytest_cache
hypothesis profile 'default'
rootdir: /home/dev/beam_dev/beam/sdks/python
configfile: pytest.ini
plugins: xdist-3.8.0, requests-mock-1.12.1, timeout-2.4.0, anyio-4.9.0, hypothesis-6.135.31
timeout: 600.0s
timeout method: signal
timeout func_only: False
collected 36 items / 27 deselected / 9 selected
milvus_search_it_test.py::TestMilvusSearchEnrichment::test_empty_input_chunks PASSED [ 11%]
milvus_search_it_test.py::TestMilvusSearchEnrichment::test_filtered_search_with_bm25_full_text_and_batching PASSED [ 22%]
milvus_search_it_test.py::TestMilvusSearchEnrichment::test_filtered_search_with_cosine_similarity_and_batching PASSED [ 33%]
milvus_search_it_test.py::TestMilvusSearchEnrichment::test_hybrid_search PASSED [ 44%]
milvus_search_it_test.py::TestMilvusSearchEnrichment::test_invalid_query_on_non_existent_collection PASSED [ 55%]
milvus_search_it_test.py::TestMilvusSearchEnrichment::test_invalid_query_on_non_existent_field PASSED [ 66%]
milvus_search_it_test.py::TestMilvusSearchEnrichment::test_keyword_search_with_inner_product_sparse_embedding PASSED [ 77%]
milvus_search_it_test.py::TestMilvusSearchEnrichment::test_vector_search_with_euclidean_distance PASSED [ 88%]
milvus_search_it_test.py::TestMilvusSearchEnrichment::test_vector_search_with_inner_product_similarity PASSED [100%]
=============================================== 9 passed, 27 deselected in 30.26s ================================================
```
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] workflows: update marker from `require_non_self_hosted` to `require_docker_in_docker` in `Pposargs` [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #35598:
URL: https://github.com/apache/beam/pull/35598#discussion_r2208330408
##########
.github/workflows/beam_PreCommit_Python_ML.yml:
##########
@@ -104,8 +104,8 @@ jobs:
arguments: |
-Pposargs="${{
contains(matrix.os, 'self-hosted') &&
- 'apache_beam/ml/ -m ''not require_non_self_hosted''' ||
- 'apache_beam/ml/ -m ''require_non_self_hosted'''
+ 'apache_beam/ml/ -m ''not require_docker_in_docker''' ||
+ 'apache_beam/ml/ -m ''require_docker_in_docker'''
Review Comment:
This is the context - https://issues.apache.org/jira/browse/BEAM-12985
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org