You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by arunmahadevan <gi...@git.apache.org> on 2017/04/10 15:23:12 UTC

[GitHub] storm pull request #2053: [STORM-2455] Expose the window start and end times...

GitHub user arunmahadevan opened a pull request:

    https://github.com/apache/storm/pull/2053

    [STORM-2455] Expose the window start and end timestamp in TupleWindow

    

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

    $ git pull https://github.com/arunmahadevan/storm STORM-2455

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

    https://github.com/apache/storm/pull/2053.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 #2053
    
----
commit 39f8bb502dbef97da9d0fb4e20f6998e95fe947d
Author: Arun Mahadevan <ar...@apache.org>
Date:   2017-04-07T08:55:21Z

    [STORM-2455] Expose the window start and end timestamp in TupleWindow

----


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

[GitHub] storm pull request #2053: [STORM-2455] Expose the window start and end times...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/storm/pull/2053


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

[GitHub] storm pull request #2053: [STORM-2455] Expose the window start and end times...

Posted by arunmahadevan <gi...@git.apache.org>.
Github user arunmahadevan commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2053#discussion_r110835582
  
    --- Diff: storm-client/src/jvm/org/apache/storm/windowing/Window.java ---
    @@ -47,9 +47,17 @@
         List<T> getExpired();
     
         /**
    -     * If processing based on event time, returns the watermark time otherwise the current timestamp.
    +     * If processing based on event time, returns the window end time based on watermark otherwise
    +     * returns the current timestamp.
    --- End diff --
    
    Thanks for noticing. Its actually returning the timestamp at which window ends for both event and processing time. I am invoking `System.currentTimeMillis` in `getEndTimestamp` which is un-necessary. Will reword the doc and refactor a bit to make it clear.


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

[GitHub] storm pull request #2053: [STORM-2455] Expose the window start and end times...

Posted by satishd <gi...@git.apache.org>.
Github user satishd commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2053#discussion_r110830274
  
    --- Diff: storm-client/src/jvm/org/apache/storm/windowing/Window.java ---
    @@ -47,9 +47,17 @@
         List<T> getExpired();
     
         /**
    -     * If processing based on event time, returns the watermark time otherwise the current timestamp.
    +     * If processing based on event time, returns the window end time based on watermark otherwise
    +     * returns the current timestamp.
    --- End diff --
    
    Returning the current timestamp(at which this method is invoked) for non event-timestamp scenarios does not look to be intuitive. Users may assume `endTimeStamp` should return the timestamp at which the window is completed. 


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

[GitHub] storm issue #2053: [STORM-2455] Expose the window start and end timestamp in...

Posted by arunmahadevan <gi...@git.apache.org>.
Github user arunmahadevan commented on the issue:

    https://github.com/apache/storm/pull/2053
  
    @harshach, the earlier change (that added getTimestamp) was checked in to only the master branch. Since its not part of any released versions of Storm may be its not an issue if we can get this merged before the 2.0 release?


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

[GitHub] storm issue #2053: [STORM-2455] Expose the window start and end timestamp in...

Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the issue:

    https://github.com/apache/storm/pull/2053
  
    +1


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

[GitHub] storm issue #2053: [STORM-2455] Expose the window start and end timestamp in...

Posted by satishd <gi...@git.apache.org>.
Github user satishd commented on the issue:

    https://github.com/apache/storm/pull/2053
  
    +1 LGTM


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

[GitHub] storm issue #2053: [STORM-2455] Expose the window start and end timestamp in...

Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the issue:

    https://github.com/apache/storm/pull/2053
  
    @arunmahadevan this looks like backward incompatible change?


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