You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "damccorm (via GitHub)" <gi...@apache.org> on 2024/09/26 14:13:56 UTC

[PR] [WIP]: Do not merge, trying to repro failure [beam]

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

   Trying to demonstrate an issue with BQ connector
   
   ------------------------
   
   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://github.com/apache/beam/blob/master/CONTRIBUTING.md#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 or the [workflows README](https://github.com/apache/beam/blob/master/.github/workflows/README.md) to see a list of phrases to trigger workflows.
   


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

Re: [PR] Allow GlobalWindows to be encoded as IntervalWindows [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #32569:
URL: https://github.com/apache/beam/pull/32569#issuecomment-2379212862

   Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment `assign set of 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

Re: [PR] Allow GlobalWindows to be encoded as IntervalWindows [beam]

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

   I'll note that there's probably a better fix here to introspect the function and figure out if it can return globally windowed things ahead of time so we can get our coders right. That's also a larger more ambiguous problem, and I'm hoping we can just get BQ IO (and other IOs which use this pattern like spanner) right for now


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

Re: [PR] Allow GlobalWindows to be encoded as IntervalWindows [beam]

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm closed pull request #32569: Allow GlobalWindows to be encoded as IntervalWindows
URL: https://github.com/apache/beam/pull/32569


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

Re: [PR] Allow GlobalWindows to be encoded as IntervalWindows [beam]

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

   Superceded by https://github.com/apache/beam/pull/32583


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

Re: [PR] Allow GlobalWindows to be encoded as IntervalWindows [beam]

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

   R: @Abacn


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

Re: [PR] Allow GlobalWindows to be encoded as IntervalWindows [beam]

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

   R: @Abacn


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

Re: [PR] Allow GlobalWindows to be encoded as IntervalWindows [beam]

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


##########
sdks/python/apache_beam/utils/windowed_value_test.py:
##########
@@ -75,6 +76,22 @@ def test_pickle(self):
     wv = windowed_value.WindowedValue(1, 3, (), pane_info)
     self.assertTrue(pickle.loads(pickle.dumps(wv)) == wv)
 
+  def test_encoding_global_window_in_interval_window(self):
+    input_data = ['123']
+
+    class ComputeWordLengthFn(beam.DoFn):
+      def process(self, element):
+        pass
+
+      def finish_bundle(self):
+        yield beam.transforms.window.GlobalWindows.windowed_value('test')

Review Comment:
   One should only emit to windows that were part of the input. 



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

Re: [PR] Allow GlobalWindows to be encoded as IntervalWindows [beam]

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


##########
sdks/python/apache_beam/coders/coder_impl.py:
##########
@@ -822,15 +824,19 @@ def _from_normal_time(self, value):
 
   def encode_to_stream(self, value, out, nested):
     # type: (IntervalWindow, create_OutputStream, bool) -> None
-    typed_value = value
+    if not TYPE_CHECKING:

Review Comment:
   We should not be checking this at every element encoding; instead lift the import.



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