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/09 17:57:22 UTC
[GitHub] [beam] lukecwik opened a new pull request, #24067: Clarify ResourceHints proto specification a little
lukecwik opened a new pull request, #24067:
URL: https://github.com/apache/beam/pull/24067
Mostly ensure that the encoding is a reference to the utf8 coder.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] 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] lukecwik commented on pull request #24067: Swap to use UTF-8 for resource hints instead of ASCII
Posted by GitBox <gi...@apache.org>.
lukecwik commented on PR #24067:
URL: https://github.com/apache/beam/pull/24067#issuecomment-1309224895
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] tvalentyn commented on pull request #24067: Swap to use UTF-8 for resource hints instead of ASCII
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on PR #24067:
URL: https://github.com/apache/beam/pull/24067#issuecomment-1309255287
I think we used ASCII due to v1beta3 JSON API limitations of what can be stored in a string inside a struct.
This may break the translation in Dataflow backend while v1beta3 is still used by Dataflow internals.
--
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 closed pull request #24067: Swap to use UTF-8 for resource hints instead of ASCII
Posted by GitBox <gi...@apache.org>.
tvalentyn closed pull request #24067: Swap to use UTF-8 for resource hints instead of ASCII
URL: https://github.com/apache/beam/pull/24067
--
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] lostluck commented on a diff in pull request #24067: Clarify ResourceHints proto specification a little
Posted by GitBox <gi...@apache.org>.
lostluck commented on code in PR #24067:
URL: https://github.com/apache/beam/pull/24067#discussion_r1018264641
##########
model/pipeline/src/main/proto/org/apache/beam/model/pipeline/v1/beam_runner_api.proto:
##########
@@ -1964,13 +1964,13 @@ message ExecutableStagePayload {
message StandardResourceHints {
enum Enum {
// Describes hardware accelerators that are desired to have in the execution environment.
- // Payload: ASCII encoded string with the following format: "type:<type>;count:<n>;<options>" where type
- // is an accelerator sku, count is the number of accelerators per worker, and options are
+ // Payload: beam:coder:string_utf8:v1 encoded string with the following format: "type:<type>;count:<n>;<options>" where type
+ // is an accelerator SKU, count is the number of accelerators per worker in the range [0, 2^64), and options are
// related options flags.
ACCELERATOR = 0 [(beam_urn) = "beam:resources:accelerator:v1"];
// Describes desired minimal available RAM size in transform's execution environment.
// SDKs should convert the size to bytes, but can allow users to specify human-friendly units (e.g. GiB).
- // Payload: ASCII encoded string of the base 10 representation of an integer number of bytes.
+ // Payload: beam:coder:string_utf8:v1 encoded string of the number of bytes in the range [0, 2^64).
Review Comment:
I think it's still worth clarifying it's in base 10, unless we are also accepting Hex or base 64 or other encodings. I'd rather be strict than allow unnecessary wiggle room. It can always be expanded later.
--
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 #24067: Swap to use UTF-8 for resource hints instead of ASCII
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #24067:
URL: https://github.com/apache/beam/pull/24067#issuecomment-1309324073
# [Codecov](https://codecov.io/gh/apache/beam/pull/24067?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 [#24067](https://codecov.io/gh/apache/beam/pull/24067?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (762b70f) into [master](https://codecov.io/gh/apache/beam/commit/539fa9159ffc116b2e79e6de2804dfdd1c1e4722?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (539fa91) will **decrease** coverage by `0.00%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #24067 +/- ##
==========================================
- Coverage 73.30% 73.30% -0.01%
==========================================
Files 714 714
Lines 96418 96418
==========================================
- Hits 70679 70678 -1
- Misses 24418 24419 +1
Partials 1321 1321
```
| Flag | Coverage Δ | |
|---|---|---|
| python | `83.19% <100.00%> (-0.01%)` | :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/24067?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/pipeline.py](https://codecov.io/gh/apache/beam/pull/24067/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcGlwZWxpbmUucHk=) | `92.25% <100.00%> (ø)` | |
| [sdks/python/apache\_beam/transforms/resources.py](https://codecov.io/gh/apache/beam/pull/24067/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9yZXNvdXJjZXMucHk=) | `93.85% <100.00%> (ø)` | |
| [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/24067/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: |
| [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/24067/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `95.12% <0.00%> (-2.44%)` | :arrow_down: |
| [...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/24067/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: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/24067/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.24%)` | :arrow_up: |
| [sdks/python/apache\_beam/transforms/combiners.py](https://codecov.io/gh/apache/beam/pull/24067/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb21iaW5lcnMucHk=) | `93.43% <0.00%> (+0.38%)` | :arrow_up: |
: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] lukecwik commented on pull request #24067: Clarify ResourceHints proto specification a little
Posted by GitBox <gi...@apache.org>.
lukecwik commented on PR #24067:
URL: https://github.com/apache/beam/pull/24067#issuecomment-1309136770
R: @lostluck @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] github-actions[bot] commented on pull request #24067: Clarify ResourceHints proto specification a little
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #24067:
URL: https://github.com/apache/beam/pull/24067#issuecomment-1309140417
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] github-actions[bot] commented on pull request #24067: Swap to use UTF-8 for resource hints instead of ASCII
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #24067:
URL: https://github.com/apache/beam/pull/24067#issuecomment-1385373711
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions.
--
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