You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "liferoad (via GitHub)" <gi...@apache.org> on 2023/01/27 19:50:52 UTC

[GitHub] [beam] liferoad opened a new pull request, #25210: Ignore flags for beam_sql magic

liferoad opened a new pull request, #25210:
URL: https://github.com/apache/beam/pull/25210

   Ignore all the flags when using beam_sql for PipelineOptions
   
   ------------------------
   
   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)
   ------------------------------------------------------------------------------------------------
   [![Build python source distribution and wheels](https://github.com/apache/beam/workflows/Build%20python%20source%20distribution%20and%20wheels/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
   [![Python tests](https://github.com/apache/beam/workflows/Python%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Java tests](https://github.com/apache/beam/workflows/Java%20Tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Go tests](https://github.com/apache/beam/workflows/Go%20tests/badge.svg?branch=master&event=schedule)](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] liferoad commented on a diff in pull request #25210: Ignore flags for beam_sql magic

Posted by "liferoad (via GitHub)" <gi...@apache.org>.
liferoad commented on code in PR #25210:
URL: https://github.com/apache/beam/pull/25210#discussion_r1091204118


##########
sdks/python/apache_beam/runners/interactive/sql/utils.py:
##########
@@ -161,7 +161,8 @@ class OptionsForm:
   generate PipelineOptions to run pipelines.
   """
   def __init__(self):
-    self.options = PipelineOptions()
+    # ignore all flags

Review Comment:
   I added more comments to explain why we propose this fix.



-- 
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 #25210: Ignore flags for beam_sql magic

Posted by "aaltay (via GitHub)" <gi...@apache.org>.
aaltay commented on PR #25210:
URL: https://github.com/apache/beam/pull/25210#issuecomment-1409750506

   I wanted to give @KevinGG a chance to complete his review. Let me know if you would like me to merge 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] aaltay commented on a diff in pull request #25210: Ignore flags for beam_sql magic

Posted by "aaltay (via GitHub)" <gi...@apache.org>.
aaltay commented on code in PR #25210:
URL: https://github.com/apache/beam/pull/25210#discussion_r1091020278


##########
sdks/python/apache_beam/runners/interactive/sql/utils.py:
##########
@@ -161,7 +161,8 @@ class OptionsForm:
   generate PipelineOptions to run pipelines.
   """
   def __init__(self):
-    self.options = PipelineOptions()
+    # ignore all flags

Review Comment:
   Could you add a link to a TODO bug, or an expanded comment explaining why this is needed here?



##########
sdks/python/apache_beam/runners/interactive/sql/utils.py:
##########
@@ -161,7 +161,8 @@ class OptionsForm:
   generate PipelineOptions to run pipelines.
   """
   def __init__(self):
-    self.options = PipelineOptions()
+    # ignore all flags
+    self.options = PipelineOptions(flags={})

Review Comment:
   Would this break any other use cases? How can we test for that?



-- 
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 a diff in pull request #25210: Ignore flags for beam_sql magic

Posted by "liferoad (via GitHub)" <gi...@apache.org>.
liferoad commented on code in PR #25210:
URL: https://github.com/apache/beam/pull/25210#discussion_r1091203555


##########
sdks/python/apache_beam/runners/interactive/sql/utils.py:
##########
@@ -161,7 +161,8 @@ class OptionsForm:
   generate PipelineOptions to run pipelines.
   """
   def __init__(self):
-    self.options = PipelineOptions()
+    # ignore all flags
+    self.options = PipelineOptions(flags={})

Review Comment:
   The current beam_sql magic tests passed. The magic does not use `flags`.



-- 
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 merged pull request #25210: Ignore flags for beam_sql magic

Posted by "aaltay (via GitHub)" <gi...@apache.org>.
aaltay merged PR #25210:
URL: https://github.com/apache/beam/pull/25210


-- 
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] KevinGG commented on a diff in pull request #25210: Ignore flags for beam_sql magic

Posted by "KevinGG (via GitHub)" <gi...@apache.org>.
KevinGG commented on code in PR #25210:
URL: https://github.com/apache/beam/pull/25210#discussion_r1092382226


##########
sdks/python/apache_beam/runners/interactive/sql/utils.py:
##########
@@ -161,7 +161,8 @@ class OptionsForm:
   generate PipelineOptions to run pipelines.
   """
   def __init__(self):
-    self.options = PipelineOptions()
+    # ignore all flags
+    self.options = PipelineOptions(flags={})

Review Comment:
   To clarify: here `flags` is a parameter of the constructor, not the flags a Beam pipeline would use.
   If not set as `{}`, the `PipelineOptions` is not initiated with a clean state and will use system's command line arguments (the current process uses, which can be anything unrelated to Beam). This is never useful in an interactive setup.



-- 
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 #25210: Ignore flags for beam_sql magic

Posted by "liferoad (via GitHub)" <gi...@apache.org>.
liferoad commented on PR #25210:
URL: https://github.com/apache/beam/pull/25210#issuecomment-1407004916

   @KevinGG Please take a look. 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] liferoad commented on a diff in pull request #25210: Ignore flags for beam_sql magic

Posted by "liferoad (via GitHub)" <gi...@apache.org>.
liferoad commented on code in PR #25210:
URL: https://github.com/apache/beam/pull/25210#discussion_r1091274166


##########
sdks/python/apache_beam/runners/interactive/sql/utils.py:
##########
@@ -161,7 +161,8 @@ class OptionsForm:
   generate PipelineOptions to run pipelines.
   """
   def __init__(self):
-    self.options = PipelineOptions()
+    # ignore all flags
+    self.options = PipelineOptions(flags={})

Review Comment:
   BTW: I forgot to mention that I also tested the fix manually by running the beam_sql magic using our Dataflow Notebooks.



-- 
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