You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by zentol <gi...@git.apache.org> on 2016/07/22 13:30:55 UTC

[GitHub] flink pull request #2286: [FLINK-4210][metrics] Move close()/isClosed() out ...

GitHub user zentol opened a pull request:

    https://github.com/apache/flink/pull/2286

    [FLINK-4210][metrics] Move close()/isClosed() out of MetricGroup

    This PR moves the `close()` and `isClosed()` methods from the MetricGroup interface into the AbstractMetricGroup class.
    
    The reasoning is that users generally shouldn't need to call it, reporters shouldn't be able to call it, and that several MetricGroup implementations don't implement the method anyway.

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

    $ git pull https://github.com/zentol/flink 4210_minus-close

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

    https://github.com/apache/flink/pull/2286.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 #2286
    
----
commit dc6e51be321eb46253100995c2b1fcc58f53aaf7
Author: zentol <ch...@apache.org>
Date:   2016-07-22T13:20:08Z

    [FLINK-4210][metrics] Move close()/isClosed() oout of MetricGroup

----


---
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] flink pull request #2286: [FLINK-4210][metrics] Move close()/isClosed() out ...

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

    https://github.com/apache/flink/pull/2286#discussion_r72217528
  
    --- Diff: flink-core/src/main/java/org/apache/flink/metrics/MetricGroup.java ---
    @@ -37,25 +37,6 @@
     public interface MetricGroup {
     
     	// ------------------------------------------------------------------------
    -	//  Closing
    -	// ------------------------------------------------------------------------
    -
    -	/**
    -	 * Marks the group as closed.
    -	 * Recursively unregisters all {@link Metric Metrics} contained in this group.
    -	 * 
    -	 * <p>Any metrics created after the call to this function will not be registered in
    -	 * the {@link MetricRegistry} and not be reported to any reporter (like JMX).
    -	 */
    -	void close();
    --- End diff --
    
    The class-level JavaDocs have a link to `close` that is broken now.


---
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] flink issue #2286: [FLINK-4210][metrics] Move close()/isClosed() out of Metr...

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

    https://github.com/apache/flink/pull/2286
  
    Looks good to merge. Following the discussion in the JIRA, I think it's nice to move it out. I had one minor inline comment and the commit message has a typo.


---
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] flink pull request #2286: [FLINK-4210][metrics] Move close()/isClosed() out ...

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

    https://github.com/apache/flink/pull/2286


---
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] flink issue #2286: [FLINK-4210][metrics] Move close()/isClosed() out of Metr...

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

    https://github.com/apache/flink/pull/2286
  
    I'll address the issues while merging, which I'm doing now.


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