Skip to content

Conversation

@kennknowles
Copy link
Member

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

  • Make sure the PR title is formatted like:
    [BEAM-<Jira issue #>] Description of pull request
  • Make sure tests pass via mvn clean verify. (Even better, enable
    Travis-CI on your fork and ensure the whole test matrix passes).
  • Replace <Jira issue #> in the title with the actual Jira issue
    number, if there is one.
  • If this contribution is large, please file an Apache
    Individual Contributor License Agreement.

A value in empty windows expands to no values, so it can be dropped at
any time, perhaps unintentionally. This has bitten runner authors, including
Spark & Dataflow.

While creating such a thing in memory is not automatically problematic, it
is also not really useful. So this change removes it.

A value in empty windows expands to no values, so it can be dropped at
any time, perhaps unintentionally. This has bitten runner authors, including
Spark & Dataflow.

While creating such a thing in memory is not automatically problematic, it
is also not really useful. So this change removes it.
@kennknowles
Copy link
Member Author

R: @bjchambers or @amitsela or really anyone.

References are this dev thread which led to #179 fixing BEAM-189. It has also be re-filed as BEAM-230.

@amitsela
Copy link
Member

amitsela commented May 10, 2016

Jenkins test is failing IT test, is it running with the DataflowPipelineRunner - meaning as a Dataflow service ? If so, I think you miss an update there..
Just to save you the time:

(ed3676eda3f9031e): java.lang.NoSuchMethodError: org.apache.beam.sdk.util.WindowedValue.valueInEmptyWindows(Ljava/lang/Object;)Lorg/apache/beam/sdk/util/WindowedValue;
at com.google.cloud.dataflow.sdk.runners.worker.GroupingShuffleReader$GroupingShuffleReaderIterator.advance(GroupingShuffleReader.java:260)

......

(9349dfed44f1940a): Workflow failed. Causes: (b529cf23ada96ef9): S04:WriteCounts/DataflowPipelineRunner.BatchTextIOWrite/Write/DataflowPipelineRunner.BatchWrite/View.AsSingleton/BatchViewAsSingleton/DataflowPipelineRunner.GroupByWindowHashAsKeyAndWindowAsSortKey/DataflowPipelineRunner.GroupByKeyAndSortValuesOnly/Read+WriteCounts/DataflowPipelineRunner.BatchTextIOWrite/Write/DataflowPipelineRunner.BatchWrite/View.AsSingleton/BatchViewAsSingleton/ParDo(IsmRecordForSingularValuePerWindow) failed.

@amitsela
Copy link
Member

amitsela commented May 10, 2016

It seems that ValueInEmptyWindows is unused now, if we don't want to remove it yet, should we @Deprecated it ? or do you have future plans for it ?

@kennknowles
Copy link
Member Author

Ah, yes, good catch. I will wipe out ValueInEmptyWindows as well. I think in our version 0.1 phase we don't need to go through @Deprecated, especially for things that are only used for runners.

@dhalperi
Copy link
Contributor

dhalperi commented Jun 8, 2016

Getting in just under 1-month-idle.. Any update here?

@kennknowles
Copy link
Member Author

Yes, there are some prerequisites that need to be addressed first.

@kennknowles
Copy link
Member Author

I am going to close this while I fix the bug in the runner.

dhalperi pushed a commit to dhalperi/beam that referenced this pull request Aug 23, 2016
iemejia pushed a commit to iemejia/beam that referenced this pull request Jan 12, 2018
pl04351820 pushed a commit to pl04351820/beam that referenced this pull request Dec 20, 2023
…uments (apache#316)

* Began refactoring away from GetDocument proto

which contains no `read_time` field and in general is a shortcut around `BatchGetDocuments`

* Correctly instantiate snapshots for missing documents

* Removed stale NotFound exception

* Removed unnecessary empty list check

* Linting fix

* Expanded batch_get change to async classes

* Updated variable name

* Added get_batch to async classes

* Improved consumption of async generators

* Fixed test coverage

* Fixed broken mock in test

* Linting

* Reverted the move of AsyncIter

Co-authored-by: Craig Labenz <craiglabenz@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants