You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "AnandInguva (via GitHub)" <gi...@apache.org> on 2023/04/03 21:00:18 UTC
[GitHub] [beam] AnandInguva opened a new pull request, #26082: Unpin tox version in BeamModulePlugin
AnandInguva opened a new pull request, #26082:
URL: https://github.com/apache/beam/pull/26082
**Please** add a meaningful description for your change here
------------------------
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://beam.apache.org/contribute/get-started-contributing/#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.
--
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
[GitHub] [beam] tvalentyn commented on pull request #26082: Unpin tox version in BeamModulePlugin
Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn commented on PR #26082:
URL: https://github.com/apache/beam/pull/26082#issuecomment-1505826636
> @tvalentyn any chance you have context on why that's been done?
I don't remember, I think tox version shouldn't be configured in ad-hoc places like that, and I would support controlled upgrades to next major version.
Other places:
```
:beam$ rg " tox==" .
./buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
2760: "pip install --retries 10 --upgrade tox==3.20.1 -r ${project.rootDir}/sdks/python/build-requirements.txt"
./sdks/python/apache_beam/io/hdfs_integration_test/Dockerfile
33:RUN pip install --no-cache-dir tox==3.11.1 -r sdks/python/build-requirements.txt
./sdks/python/apache_beam/io/azure/integration_test/Dockerfile
35:RUN pip install --no-cache-dir tox==3.11.1 -r sdks/python/build-requirements.txt
:beam$
```
--
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
[GitHub] [beam] AnandInguva commented on pull request #26082: Unpin tox version in Beam code base
Posted by "AnandInguva (via GitHub)" <gi...@apache.org>.
AnandInguva commented on PR #26082:
URL: https://github.com/apache/beam/pull/26082#issuecomment-1578948582
Run Python_Runners PreCommit
--
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
[GitHub] [beam] liferoad commented on pull request #26082: Unpin tox version in Beam code base
Posted by "liferoad (via GitHub)" <gi...@apache.org>.
liferoad commented on PR #26082:
URL: https://github.com/apache/beam/pull/26082#issuecomment-1578950500
I think pining this to a specific version is better.
--
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
[GitHub] [beam] AnandInguva commented on pull request #26082: Unpin tox version in BeamModulePlugin
Posted by "AnandInguva (via GitHub)" <gi...@apache.org>.
AnandInguva commented on PR #26082:
URL: https://github.com/apache/beam/pull/26082#issuecomment-1494972879
R: @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
[GitHub] [beam] AnandInguva commented on a diff in pull request #26082: Unpin tox version in Beam code base
Posted by "AnandInguva (via GitHub)" <gi...@apache.org>.
AnandInguva commented on code in PR #26082:
URL: https://github.com/apache/beam/pull/26082#discussion_r1218709114
##########
sdks/python/tox.ini:
##########
@@ -40,6 +40,8 @@ allowlist_externals =
time
bash
rm
+ /bin/sh
+ curl
Review Comment:
```suggestion
curl
chmod
```
--
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
[GitHub] [beam] AnandInguva commented on pull request #26082: Unpin tox version in BeamModulePlugin
Posted by "AnandInguva (via GitHub)" <gi...@apache.org>.
AnandInguva commented on PR #26082:
URL: https://github.com/apache/beam/pull/26082#issuecomment-1496019233
> We may end up wanting to pin to a 4.x version (probably latest), but I don't see any obvious problems with unpinning entirely - I just know historically that's what we've done.
>
> @tvalentyn any chance you have context on why that's been done?
we could pin `tox<5` so that we won't update to next major version automatically and leading to breaking changes
--
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
[GitHub] [beam] tvalentyn commented on pull request #26082: Unpin tox version in BeamModulePlugin
Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn commented on PR #26082:
URL: https://github.com/apache/beam/pull/26082#issuecomment-1507697992
FWIW, tox gives a headsup about breaking changes:
```
8839 test_1 | WARNING: test command found but not installed in testenv
8840 test_1 | cmd: /bin/bash
8841 test_1 | env: /app/sdks/python/target/.tox/azure_integration_test
8842 test_1 | Maybe you forgot to specify a dependency? See also the whitelist_externals envconfig setting.
8843 test_1 |
8844 test_1 | DEPRECATION WARNING: this will be an error in tox 4 and above!
8845 test_1 | ___________________________________ summary ______________________________
```
--
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
[GitHub] [beam] AnandInguva commented on pull request #26082: Unpin tox version in Beam code base
Posted by "AnandInguva (via GitHub)" <gi...@apache.org>.
AnandInguva commented on PR #26082:
URL: https://github.com/apache/beam/pull/26082#issuecomment-1518306873
Run Python_Coverage PreCommit
--
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
[GitHub] [beam] AnandInguva commented on pull request #26082: Unpin tox version in BeamModulePlugin
Posted by "AnandInguva (via GitHub)" <gi...@apache.org>.
AnandInguva commented on PR #26082:
URL: https://github.com/apache/beam/pull/26082#issuecomment-1496019938
> Looks like this is causing CI to fail as well.
yes, I will take a look. I will ping on the PR once they are resolved
--
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
[GitHub] [beam] AnandInguva commented on a diff in pull request #26082: Unpin tox version in Beam code base
Posted by "AnandInguva (via GitHub)" <gi...@apache.org>.
AnandInguva commented on code in PR #26082:
URL: https://github.com/apache/beam/pull/26082#discussion_r1218745431
##########
sdks/python/tox.ini:
##########
@@ -40,6 +40,9 @@ allowlist_externals =
time
bash
rm
+ /bin/sh
+ curl
Review Comment:
```suggestion
curl
./codecov
```
--
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
[GitHub] [beam] AnandInguva commented on pull request #26082: Unpin tox version in Beam code base
Posted by "AnandInguva (via GitHub)" <gi...@apache.org>.
AnandInguva commented on PR #26082:
URL: https://github.com/apache/beam/pull/26082#issuecomment-1577828367
R: @tvalentyn
--
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
[GitHub] [beam] AnandInguva commented on pull request #26082: Unpin tox version in Beam code base
Posted by "AnandInguva (via GitHub)" <gi...@apache.org>.
AnandInguva commented on PR #26082:
URL: https://github.com/apache/beam/pull/26082#issuecomment-1578948338
Run Python_Integration PreCommit
--
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
[GitHub] [beam] AnandInguva commented on pull request #26082: Unpin tox version in BeamModulePlugin
Posted by "AnandInguva (via GitHub)" <gi...@apache.org>.
AnandInguva commented on PR #26082:
URL: https://github.com/apache/beam/pull/26082#issuecomment-1509005500
> > Looks like this is causing CI to fail as well.
>
> > yes, I will take a look. I will ping on the PR once they are resolved
>
> not seeing tox-related errors.
it was tox error. `/bin/bash` needs to be added to allowlist_externals list. Lets see if it resolves the error.
--
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
[GitHub] [beam] github-actions[bot] commented on pull request #26082: Unpin tox version in BeamModulePlugin
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #26082:
URL: https://github.com/apache/beam/pull/26082#issuecomment-1494973937
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control
--
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
[GitHub] [beam] tvalentyn commented on pull request #26082: Unpin tox version in BeamModulePlugin
Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn commented on PR #26082:
URL: https://github.com/apache/beam/pull/26082#issuecomment-1505879634
I searched around and it doesn't seem common to specify tox upper bound requirements in codebases.
--
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
[GitHub] [beam] tvalentyn commented on pull request #26082: Unpin tox version in BeamModulePlugin
Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn commented on PR #26082:
URL: https://github.com/apache/beam/pull/26082#issuecomment-1505880711
> Looks like this is causing CI to fail as well.
> yes, I will take a look. I will ping on the PR once they are resolved
not seeing tox-related errors.
--
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
[GitHub] [beam] AnandInguva commented on pull request #26082: Unpin tox version in Beam code base
Posted by "AnandInguva (via GitHub)" <gi...@apache.org>.
AnandInguva commented on PR #26082:
URL: https://github.com/apache/beam/pull/26082#issuecomment-1578958868
> I think pining this to a specific version is better.
I followed the pattern of github tests where we didn't pin the version https://github.com/apache/beam/blob/70eae86fc35803fd8f23e7e8c3cfda0250266d86/.github/workflows/python_tests.yml#L121.
I would like to be consistent across the code base. we can do `tox<5.0.0` everywhere in the code base
--
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
[GitHub] [beam] AnandInguva merged pull request #26082: Unpin tox version in Beam code base
Posted by "AnandInguva (via GitHub)" <gi...@apache.org>.
AnandInguva merged PR #26082:
URL: https://github.com/apache/beam/pull/26082
--
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
[GitHub] [beam] codecov[bot] commented on pull request #26082: Unpin tox version in Beam code base
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #26082:
URL: https://github.com/apache/beam/pull/26082#issuecomment-1577748650
## [Codecov](https://app.codecov.io/gh/apache/beam/pull/26082?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#26082](https://app.codecov.io/gh/apache/beam/pull/26082?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (055e0cc) into [master](https://app.codecov.io/gh/apache/beam/commit/3dc07743b4e4cecd959760fd0d7484affeed34e3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (3dc0774) will **decrease** coverage by `0.04%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #26082 +/- ##
==========================================
- Coverage 71.51% 71.48% -0.04%
==========================================
Files 768 853 +85
Lines 103725 103985 +260
==========================================
+ Hits 74184 74333 +149
- Misses 27992 28103 +111
Partials 1549 1549
```
| Flag | Coverage Δ | |
|---|---|---|
| python | `81.03% <ø> (-0.10%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
[see 140 files with indirect coverage changes](https://app.codecov.io/gh/apache/beam/pull/26082/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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
[GitHub] [beam] damccorm commented on pull request #26082: Unpin tox version in BeamModulePlugin
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #26082:
URL: https://github.com/apache/beam/pull/26082#issuecomment-1496016255
Looks like this is causing CI to fail as well.
--
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