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/06/14 20:57:09 UTC
[GitHub] [beam] waltage opened a new pull request, #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
waltage opened a new pull request, #21867:
URL: https://github.com/apache/beam/pull/21867
### **Python - IO - GCP - BigQuery:**
Promotes **_schema update options_** that are currently available as entries in the `additional_bq_parameters` dict to named and verified arguments (similar to the current Write and Update Dispositions).
addresses #21141
------------------------
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/#make-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)
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] github-actions[bot] commented on pull request #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1183841187
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] asf-ci commented on pull request #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1155705463
Can one of the admins verify this patch?
--
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 #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1582513033
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
[GitHub] [beam] ahmedabu98 commented on a diff in pull request #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by GitBox <gi...@apache.org>.
ahmedabu98 commented on code in PR #21867:
URL: https://github.com/apache/beam/pull/21867#discussion_r931244462
##########
sdks/python/apache_beam/io/gcp/bigquery_tools.py:
##########
@@ -527,6 +531,7 @@ def _insert_load_job(
schema=job_schema,
writeDisposition=write_disposition,
createDisposition=create_disposition,
+ schemaUpdateOptions=schema_update_options,
sourceFormat=source_format,
useAvroLogicalTypes=True,
autodetect=schema == 'SCHEMA_AUTODETECT',
Review Comment:
We should confirm early in WriteToBigQuery that the schema update options are present in either `schema_update_options` or `additional_load_parameters` and resolve if options are present in both.
If options are present in `additional_load_parameters` at this point, we would have two `schemaUpdateOptions` parameters here.
##########
sdks/python/apache_beam/io/gcp/bigquery_tools.py:
##########
@@ -527,6 +531,7 @@ def _insert_load_job(
schema=job_schema,
writeDisposition=write_disposition,
createDisposition=create_disposition,
+ schemaUpdateOptions=schema_update_options,
sourceFormat=source_format,
useAvroLogicalTypes=True,
autodetect=schema == 'SCHEMA_AUTODETECT',
Review Comment:
We should also allow backwards compatibility. If users want to keep using `additional_load_parameters` instead to specify update options, would they be able to?
--
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] waltage commented on pull request #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by GitBox <gi...@apache.org>.
waltage commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1187593349
@pabloem no worries on the review timeline.
I am not the original reporter of the Jira issue, but it looks like this particular feature was requested with an objective of bringing the Python API closer in line with Java API. Since the issue was ported over to github and tagged with first issue, and since Ken in the original issue said it seemed like a good idea, I went ahead and worked on implementing it here.
I agree that this particular initializer is quite dense, regardless of whether this option is promoted to a parameter. I think there are a few ways I could go with this:
1. (assuming I can update the other unit tests that use this transform, and provide sufficient integration coverage) Allow this option promotion in light of the already dense parameter set.
2. Develop type and value checking for the ...bq_parameters... dict, which would allow the surface of the initializer to remain consistent.
3. Close the issue/pr, as interest looks like it has subsided.
4. Abstract away a **majority** of the transform's initializer parameters that are BigQuery-specific into a new type/value-checked Class that would allow more flexibility for changes like this (or changes to the BigQuery APIs themselves) without modifying existing surfaces of the transform itself.
In a related note, as a tie-breaker for 1 & 2 & 3, and in support of the design of 4 above, does the BEAM team currently analyze any of the public usages of its python libraries? I can look at how this transform is used "in the wild" and try to understand how often parameters are used vs. what keys of the dict are typically set (e.g., at first glance, it looks like partitioning specs are by far the most commonly used keys in the ...bq_parameters... dict).
I'm happy to work on any of the above, just let me know how best to proceed.
--
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] waltage commented on pull request #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by GitBox <gi...@apache.org>.
waltage commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1171795673
following up on the status of this review
--
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] asf-ci commented on pull request #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1155705464
Can one of the admins verify this patch?
--
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] waltage commented on pull request #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by GitBox <gi...@apache.org>.
waltage commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1185057642
> This looks structurally fine to me. My only question is if we have an integration test suite that could be expanded to include a test that leverages these parameter, to verify they work as expected
If you could point me in the right direction (like a particular directory or a test file), I can work on adding one.
My original thinking was that any integration testing on this particular "feature" would reduce to a change-detector test given that the implementation is setting the parameter on the proto request itself ([here](https://github.com/apache/beam/pull/21867/commits/4083fa49cb6e9955b2f7eca70727c8571b5652ed#diff-5427a5d3887eb695cefde082c58575a2372972996b547d55961abeb4f7bc3debR534)), but it is duly noted that the string values themselves are not explicitly being tested anywhere
--
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] asf-ci commented on pull request #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1155705465
Can one of the admins verify this patch?
--
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] closed pull request #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
URL: https://github.com/apache/beam/pull/21867
--
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] ahmedabu98 commented on pull request #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by GitBox <gi...@apache.org>.
ahmedabu98 commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1213290639
FYI would need to see what existing checks are performed against `additional_parameters["schemaUpdateOptions"]` and make sure the new `schemaUpdateOptions` argument is there too. e.g. [here](https://github.com/apache/beam/blob/7a9bb76fe9f4c167c1d125db9d2cff9a1a315149/sdks/python/apache_beam/io/gcp/bigquery_file_loads.py#L380-L384).
--
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] asf-ci commented on pull request #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1155705461
Can one of the admins verify this patch?
--
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] waltage commented on pull request #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by GitBox <gi...@apache.org>.
waltage commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1156743855
The next closest OWNERS file is from the IO module, so please excuse me if this is an incorrect reviewer choice:
R: @aaltay
--
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 #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1155782405
# [Codecov](https://codecov.io/gh/apache/beam/pull/21867?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 [#21867](https://codecov.io/gh/apache/beam/pull/21867?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5c45a5f) into [master](https://codecov.io/gh/apache/beam/commit/4b33a38cdd06bebb9ab1b0a04844944937a8bd8b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4b33a38) will **decrease** coverage by `0.00%`.
> The diff coverage is `96.42%`.
```diff
@@ Coverage Diff @@
## master #21867 +/- ##
==========================================
- Coverage 74.07% 74.07% -0.01%
==========================================
Files 698 698
Lines 92574 92627 +53
==========================================
+ Hits 68577 68613 +36
- Misses 22742 22759 +17
Partials 1255 1255
```
| Flag | Coverage Δ | |
|---|---|---|
| python | `83.75% <96.42%> (-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/21867?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/gcp/bigquery.py](https://codecov.io/gh/apache/beam/pull/21867/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2JpZ3F1ZXJ5LnB5) | `71.00% <95.65%> (+0.93%)` | :arrow_up: |
| [...s/python/apache\_beam/io/gcp/bigquery\_file\_loads.py](https://codecov.io/gh/apache/beam/pull/21867/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2JpZ3F1ZXJ5X2ZpbGVfbG9hZHMucHk=) | `87.79% <100.00%> (+0.08%)` | :arrow_up: |
| [sdks/python/apache\_beam/io/gcp/bigquery\_tools.py](https://codecov.io/gh/apache/beam/pull/21867/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2JpZ3F1ZXJ5X3Rvb2xzLnB5) | `85.69% <100.00%> (+0.03%)` | :arrow_up: |
| [.../python/apache\_beam/transforms/periodicsequence.py](https://codecov.io/gh/apache/beam/pull/21867/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9wZXJpb2RpY3NlcXVlbmNlLnB5) | `96.77% <0.00%> (-1.59%)` | :arrow_down: |
| [sdks/python/apache\_beam/io/source\_test\_utils.py](https://codecov.io/gh/apache/beam/pull/21867/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vc291cmNlX3Rlc3RfdXRpbHMucHk=) | `88.01% <0.00%> (-1.39%)` | :arrow_down: |
| [...eam/runners/portability/fn\_api\_runner/execution.py](https://codecov.io/gh/apache/beam/pull/21867/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL2V4ZWN1dGlvbi5weQ==) | `92.44% <0.00%> (-0.65%)` | :arrow_down: |
| [sdks/python/apache\_beam/pipeline.py](https://codecov.io/gh/apache/beam/pull/21867/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=) | `91.80% <0.00%> (ø)` | |
| [sdks/python/apache\_beam/utils/urns.py](https://codecov.io/gh/apache/beam/pull/21867/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvdXJucy5weQ==) | `88.70% <0.00%> (ø)` | |
| [sdks/python/apache\_beam/coders/coders.py](https://codecov.io/gh/apache/beam/pull/21867/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vY29kZXJzL2NvZGVycy5weQ==) | `88.22% <0.00%> (ø)` | |
| [sdks/python/apache\_beam/runners/common.py](https://codecov.io/gh/apache/beam/pull/21867/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9jb21tb24ucHk=) | `88.68% <0.00%> (ø)` | |
| ... and [39 more](https://codecov.io/gh/apache/beam/pull/21867/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/21867?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/21867?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [4b33a38...5c45a5f](https://codecov.io/gh/apache/beam/pull/21867?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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 #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1682215033
This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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] pabloem commented on pull request #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by GitBox <gi...@apache.org>.
pabloem commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1185766365
hi @waltage I am so sorry to be so delayed on this. I prefer to avoid adding new parameters to the transform - it already has lots and lots of parameters, and it's hard to be sure which ones to fill up.
I can see it's valuable to be able to validate the parameter directly, so that's fair. (although we could potentially validate it within`additional_bq_parameters`)
Can you tell me more about the reasoning to add it?
--
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 #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by GitBox <gi...@apache.org>.
johnjcasey commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1184485375
This looks structurally fine to me. My only question is if we have an integration test suite that could be expanded to include a test that leverages these parameter, to verify they work as expected
--
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] waltage commented on pull request #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by GitBox <gi...@apache.org>.
waltage commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1181982637
@aaltay @pabloem
I recognize this is a lower priority issue/PR, but it's my first PR with Apache and on Beam. I'd like to move on to some higher priority issues for you all, but this review is currently a blocker
--
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] pabloem commented on pull request #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by GitBox <gi...@apache.org>.
pabloem commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1192750892
thanks for the explanation @waltage - I am going to be on vacation for a little while, so I can't advice at the moment.
@ahmedabu98 @Abacn @johnjcasey could y'all help Daniel figure out how to move forward? thanks
--
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] ahmedabu98 commented on pull request #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by GitBox <gi...@apache.org>.
ahmedabu98 commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1196967689
Agree with @johnjcasey that we need an integration test for this change, specifically to make sure that both `additional_load_parameters` and `schema_update_options` are viable. I would be happy to review that test.
@waltage we have a few BQ tests in [apache_beam/io/gcp](https://github.com/apache/beam/tree/master/sdks/python/apache_beam/io/gcp). This is an [example test](https://github.com/apache/beam/blob/82384a462a04c046723d8dcd8377373ecc5767d4/sdks/python/apache_beam/io/gcp/bigquery_test.py#L1774-L1842) that uses [`BigQueryTableMatcher`](https://github.com/apache/beam/blob/c36f54486a5185fd11bc34186a688e9b7abfa4b5/sdks/python/apache_beam/io/gcp/tests/bigquery_matcher.py#L207) to test for options in `additional_bq_parameters`.
--
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] asf-ci commented on pull request #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1155705471
Can one of the admins verify this patch?
--
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] aaltay commented on pull request #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by GitBox <gi...@apache.org>.
aaltay commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1183840702
R: @pabloem @johnjcasey - could one of you please review?
@waltage - could you please fix the test errors in the mean time?
--
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 #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by GitBox <gi...@apache.org>.
johnjcasey commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1196891357
I am ok with the dense parameter set. The Beam convention is to use named parameters in python like we use builder parameters in Java. If something has a default, that default must be valid such that the user doesn't need to provide it.
I like brining python in line with Java, and I prefer having a named parameter to a dict for additional parameters
--
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 #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1454728204
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
[GitHub] [beam] github-actions[bot] commented on pull request #21867: Beam 12994 - Python SDK BigQuery - Promote `schemaUpdateOptions` to named arguments
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #21867:
URL: https://github.com/apache/beam/pull/21867#issuecomment-1671255343
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