beam-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kennknowles <...@git.apache.org>
Subject [GitHub] incubator-beam pull request: Various fixes surrounding OutputTimeF...
Date Tue, 24 May 2016 20:13:27 GMT
GitHub user kennknowles opened a pull request:

    https://github.com/apache/incubator-beam/pull/385

    Various fixes surrounding OutputTimeFn

    Be sure to do all of the following to help us incorporate your contribution
    quickly and easily:
    
     - [x] Make sure the PR title is formatted like:
       `[BEAM-<Jira issue #>] Description of pull request`
     - [x] Make sure tests pass via `mvn clean verify`. (Even better, enable
           Travis-CI on your fork and ensure the whole test matrix passes).
     - [x] Replace `<Jira issue #>` in the title with the actual Jira issue
           number, if there is one.
     - [x] If this contribution is large, please file an Apache
           [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.txt).
    
    ---
    
    These are some bugs left behind by changes to `OutputTimeFn`.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/kennknowles/incubator-beam OutputTimeFn

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-beam/pull/385.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #385
    
----
commit a20786d42b8707504bfc1ffc71f8af71843cfcd6
Author: Kenneth Knowles <klk@google.com>
Date:   2016-05-24T20:06:31Z

    Improve the inheritance API for OutputTimeFn
    
    Previously, OutputTimeFn.Defaults enforced that any subclass *could not*
    return true for dependsOnlyOnWindow(), to encourage such subclasses to
    extend OutputTimeFn.DependsOnlyOnWindow. Unfortunately, this is at odds
    with compositional style, where all methods need to be forwarded.
    
    Further, OutputTimeFn itself was only instantiable via one of these
    subclasses, so there was no way to implement a proper forwarding.
    
    After this change, the API from OutputTimeFn to its subclasses is
    that of a simpler abstract superclass, and Defaults no longer restricts
    its subclasses.
    
    Note that we intend to remove OutputTimeFn as a userland function anyhow,
    replacing it with an enum and OutputTimeFn implementations in runners/core,
    so this is low risk. It is just to unblock some current work.

commit d94a6f15cf9f606495f5dfb1723333cfba5217f9
Author: Kenneth Knowles <klk@google.com>
Date:   2016-05-24T20:10:38Z

    Correctly forward dependsOnlyOnWindow(), etc, in WindowingStrategy
    
    Previously, WindowingStrategy contained an implemenation of OutputTimeFn
    that did not properly forward these important methods.

commit 55aae464530414f6e3ebe1103c32e39e6fc98a6f
Author: Kenneth Knowles <klk@google.com>
Date:   2016-05-24T20:11:40Z

    Fixes to timestamps in GroupAlsoByWindowsProperties
    
    These properties had poor test coverage, so their timestamps were not
    updated alongside the new default for OutputTimeFn.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message