You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/11/27 12:47:15 UTC
[GitHub] [beam] iht opened a new pull request, #24363: Minor changes to Python fileio
iht opened a new pull request, #24363:
URL: https://github.com/apache/beam/pull/24363
This PR fixes #24362 and adds a slight change to the documentation, to make it clear that the `file_naming` function also receives `destination` as a parameter (it is mentioned at the beginning of the documentation page, but in `WriteToFiles` that parameter description is missing).
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] Abacn commented on pull request #24363: Minor changes to Python fileio
Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #24363:
URL: https://github.com/apache/beam/pull/24363#issuecomment-1374607627
thanks for the interests. I would expect many places to change and imo limited benefit compared to the risk.
--
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] johnjcasey commented on pull request #24363: Minor changes to Python fileio
Posted by GitBox <gi...@apache.org>.
johnjcasey commented on PR #24363:
URL: https://github.com/apache/beam/pull/24363#issuecomment-1329795745
Agreed with Yi. Can you add a note to the changes file?
--
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] Abacn commented on pull request #24363: Minor changes to Python fileio
Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #24363:
URL: https://github.com/apache/beam/pull/24363#issuecomment-1329836316
I also suspect same issue also exists elsewhere in the code base. e.g. here: https://github.com/apache/beam/blob/3477d4a76d46c1ad3c89acb636ea7807bfc23e15/sdks/python/apache_beam/io/filebasedsink.py#L208
https://github.com/apache/beam/blob/3477d4a76d46c1ad3c89acb636ea7807bfc23e15/sdks/java/core/src/main/java/org/apache/beam/sdk/io/ShardNameTemplate.java#L57
unfortunately changing all these would be tedious...
--
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] johnjcasey commented on pull request #24363: Minor changes to Python fileio
Posted by GitBox <gi...@apache.org>.
johnjcasey commented on PR #24363:
URL: https://github.com/apache/beam/pull/24363#issuecomment-1329316387
run python 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] Abacn commented on pull request #24363: Minor changes to Python fileio
Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #24363:
URL: https://github.com/apache/beam/pull/24363#issuecomment-1330793512
Sorry, I mean the finding is great, however due to same issue elsewhere, partial fix would then make the indexing inconsistent throughout 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] codecov[bot] commented on pull request #24363: Minor changes to Python fileio
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #24363:
URL: https://github.com/apache/beam/pull/24363#issuecomment-1328268952
# [Codecov](https://codecov.io/gh/apache/beam/pull/24363?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#24363](https://codecov.io/gh/apache/beam/pull/24363?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27b3601) into [master](https://codecov.io/gh/apache/beam/commit/fd7e86269fa7d7827c2799b613eeea094a070491?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fd7e862) will **decrease** coverage by `0.01%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #24363 +/- ##
==========================================
- Coverage 73.36% 73.35% -0.02%
==========================================
Files 718 718
Lines 97034 97034
==========================================
- Hits 71188 71177 -11
- Misses 24516 24527 +11
Partials 1330 1330
```
| Flag | Coverage Δ | |
|---|---|---|
| python | `82.98% <100.00%> (-0.02%)` | :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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/24363?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/io/fileio.py](https://codecov.io/gh/apache/beam/pull/24363/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZmlsZWlvLnB5) | `96.05% <100.00%> (ø)` | |
| [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/24363/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdGVzdGluZy90ZXN0X3N0cmVhbV9zZXJ2aWNlLnB5) | `88.09% <0.00%> (-4.77%)` | :arrow_down: |
| [...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/24363/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9ydW5uZXIucHk=) | `90.50% <0.00%> (-1.27%)` | :arrow_down: |
| [sdks/python/apache\_beam/io/localfilesystem.py](https://codecov.io/gh/apache/beam/pull/24363/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vbG9jYWxmaWxlc3lzdGVtLnB5) | `90.97% <0.00%> (-0.76%)` | :arrow_down: |
| [sdks/python/apache\_beam/runners/direct/executor.py](https://codecov.io/gh/apache/beam/pull/24363/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kaXJlY3QvZXhlY3V0b3IucHk=) | `96.46% <0.00%> (-0.55%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/24363/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `89.24% <0.00%> (-0.33%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/24363/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.54% <0.00%> (-0.25%)` | :arrow_down: |
: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=The+Apache+Software+Foundation)
--
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 #24363: Minor changes to Python fileio
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #24363:
URL: https://github.com/apache/beam/pull/24363#issuecomment-1328295734
Assigning reviewers. If you would like to opt out of this review, comment `assign to next reviewer`:
R: @AnandInguva for label python.
R: @johnjcasey for label io.
Available commands:
- `stop reviewer notifications` - opt out of the automated review tooling
- `remind me after tests pass` - tag the comment author after tests pass
- `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
The PR bot will only process comments in the main thread (not review 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
[GitHub] [beam] iht commented on pull request #24363: Minor changes to Python fileio
Posted by GitBox <gi...@apache.org>.
iht commented on PR #24363:
URL: https://github.com/apache/beam/pull/24363#issuecomment-1378519732
>
> thanks for the interests. I would expect many places to change and imo limited benefit compared to the risk.
I am going to close this PR now, and I will be sending a new one with consistent changes between the Python and Java SDKs. After the holidays, this branch is outdated, and I prefer just to start over than resolving the conflcits.
--
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] iht closed pull request #24363: Minor changes to Python fileio
Posted by GitBox <gi...@apache.org>.
iht closed pull request #24363: Minor changes to Python fileio
URL: https://github.com/apache/beam/pull/24363
--
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 #24363: Minor changes to Python fileio
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #24363:
URL: https://github.com/apache/beam/pull/24363#issuecomment-1346371661
Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment `assign to next reviewer`:
R: @tvalentyn for label python.
R: @chamikaramj for label io.
Available commands:
- `stop reviewer notifications` - opt out of the automated review tooling
- `remind me after tests pass` - tag the comment author after tests pass
- `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
--
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 #24363: Minor changes to Python fileio
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #24363:
URL: https://github.com/apache/beam/pull/24363#issuecomment-1342637426
Reminder, please take a look at this pr: @AnandInguva @johnjcasey
--
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] johnjcasey commented on pull request #24363: Minor changes to Python fileio
Posted by GitBox <gi...@apache.org>.
johnjcasey commented on PR #24363:
URL: https://github.com/apache/beam/pull/24363#issuecomment-1329316675
LGTM, just rerunning python
--
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] Abacn commented on pull request #24363: Minor changes to Python fileio
Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #24363:
URL: https://github.com/apache/beam/pull/24363#issuecomment-1329765727
This makes more sense, obviously. Just have worried that if, ever, some user apps could assume the (00123-of-00124) naming pattern and we will break them mysteriously. It may deserve to be noted in "breaking change".
--
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 #24363: Minor changes to Python fileio
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #24363:
URL: https://github.com/apache/beam/pull/24363#issuecomment-1346689829
Stopping reviewer notifications for this pull request: requested by reviewer
--
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 #24363: Minor changes to Python fileio
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on PR #24363:
URL: https://github.com/apache/beam/pull/24363#issuecomment-1346688164
stop reviewer notifications
--
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] iht commented on pull request #24363: Minor changes to Python fileio
Posted by GitBox <gi...@apache.org>.
iht commented on PR #24363:
URL: https://github.com/apache/beam/pull/24363#issuecomment-1332138388
> Sorry, I mean the finding is great, however due to same issue elsewhere, partial fix would then make the indexing inconsistent throughout the code base.
Ah, ok. I have checked that this is used in fileio and in the dataframes module. I will update this PR with more details about the potential impact. So far, in my tests, this does not break anything anywhere else in the Python SDK.
Or do you mean that the behavior should be the same in the Java and Python SDKs? I can also add a similar change to the Java SDK, and check where is that used in the rest of the SDK to evaluate potential impact.
--
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] Abacn commented on pull request #24363: Minor changes to Python fileio
Posted by GitBox <gi...@apache.org>.
Abacn commented on PR #24363:
URL: https://github.com/apache/beam/pull/24363#issuecomment-1332422291
> Or do you mean that the behavior should be the same in the Java and Python SDKs? I can also add a similar change to the Java SDK, and check where is that used in the rest of the SDK to evaluate potential impact.
Yes, ideally naming should be consistent throughout SDKs. Currently it is consistent (though weird). Would appreciate if you are willing to find the appearance in both the Java and Python SDKs.
--
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] ZhengLin-Li commented on pull request #24363: Minor changes to Python fileio
Posted by GitBox <gi...@apache.org>.
ZhengLin-Li commented on PR #24363:
URL: https://github.com/apache/beam/pull/24363#issuecomment-1374374430
> > Or do you mean that the behavior should be the same in the Java and Python SDKs? I can also add a similar change to the Java SDK, and check where is that used in the rest of the SDK to evaluate potential impact.
>
> Yes, ideally naming should be consistent throughout SDKs. Currently it is consistent (though weird). Would appreciate if you are willing to find the appearance in both the Java and Python SDKs.
@Abacn
I'd like to keep working on and change the Go and Java SDK. What should I do with Git? Should I create a new branch from master? Seams that I can not create a branch based on this branch([iht:minor_python_doc](https://github.com/iht/beam/tree/minor_python_doc))
--
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