You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by greghogan <gi...@git.apache.org> on 2017/06/23 01:12:37 UTC

[GitHub] flink pull request #4170: [FLINK-6358] [gelly] Write job details for Gelly e...

GitHub user greghogan opened a pull request:

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

    [FLINK-6358] [gelly] Write job details for Gelly examples

    Add an option to write job details to a file in JSON format. Job details include: job ID, runtime, parameters with values, and accumulators with values.

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

    $ git pull https://github.com/greghogan/flink 6358_write_job_details_for_gelly_examples

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

    https://github.com/apache/flink/pull/4170.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 #4170
    
----
commit f7a89bc9e5ac5e9b34dd05db1cdcdde4e3a9882d
Author: Greg Hogan <co...@greghogan.com>
Date:   2017-06-21T14:25:57Z

    [FLINK-6358] [gelly] Write job details for Gelly examples
    
    Add an option to write job details to a file in JSON format. Job details
    include: job ID, runtime, parameters with values, and accumulators with
    values.

----


---
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 #4170: [FLINK-6358] [gelly] Write job details for Gelly e...

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

    https://github.com/apache/flink/pull/4170#discussion_r124544570
  
    --- Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/Runner.java ---
    @@ -103,6 +113,18 @@
     		.addClass(PageRank.class)
     		.addClass(TriangleListing.class);
     
    +	private ParameterTool parameters;
    +
    +	private BooleanParameter disableObjectReuse = new BooleanParameter(this, "__disable_object_reuse");
    +
    +	private StringParameter jobDetails = new StringParameter(this, "__write_job_details")
    --- End diff --
    
    `private final`?


---
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 #4170: [FLINK-6358] [gelly] Write job details for Gelly e...

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

    https://github.com/apache/flink/pull/4170#discussion_r124547670
  
    --- Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/Runner.java ---
    @@ -305,6 +334,41 @@ public static void main(String[] args) throws Exception {
     			default:
     				throw new ProgramParametrizationException("Unknown output type: " + outputName);
     		}
    +
    +		// ----------------------------------------------------------------------------------------
    +		// Write job details
    --- End diff --
    
    It would nicer to extract this if condition to `writeJobDetails` method and delete this comment


---
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 #4170: [FLINK-6358] [gelly] Write job details for Gelly e...

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

    https://github.com/apache/flink/pull/4170#discussion_r124545656
  
    --- Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/Runner.java ---
    @@ -103,6 +113,18 @@
     		.addClass(PageRank.class)
     		.addClass(TriangleListing.class);
     
    +	private ParameterTool parameters;
    +
    +	private BooleanParameter disableObjectReuse = new BooleanParameter(this, "__disable_object_reuse");
    +
    +	private StringParameter jobDetails = new StringParameter(this, "__write_job_details")
    +		.setDefaultValue("");
    +
    +	@Override
    +	public String getName() {
    +		return this.getClass().getSimpleName();
    --- End diff --
    
    Is a `Runner` good value to be returned to the user? Is this used anywhere?


---
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 #4170: [FLINK-6358] [gelly] Write job details for Gelly e...

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

    https://github.com/apache/flink/pull/4170#discussion_r126312916
  
    --- Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/Runner.java ---
    @@ -103,6 +113,18 @@
     		.addClass(PageRank.class)
     		.addClass(TriangleListing.class);
     
    +	private ParameterTool parameters;
    +
    +	private BooleanParameter disableObjectReuse = new BooleanParameter(this, "__disable_object_reuse");
    --- End diff --
    
    Done.


---
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 #4170: [FLINK-6358] [gelly] Write job details for Gelly e...

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

    https://github.com/apache/flink/pull/4170#discussion_r124547387
  
    --- Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/Runner.java ---
    @@ -70,7 +79,8 @@
      * <p>Algorithms must explicitly support each type of output via implementation of
      * interfaces. This is scalable as the number of outputs is small and finite.
      */
    -public class Runner {
    +public class Runner
    +extends ParameterizedBase {
    --- End diff --
    
    why do you extend `ParametrizedBase`? Isn't it intended as a base class for parameters? Do you use `Runner` in such context?


---
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 #4170: [FLINK-6358] [gelly] Write job details for Gelly e...

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

    https://github.com/apache/flink/pull/4170#discussion_r126312909
  
    --- Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/Runner.java ---
    @@ -103,6 +113,18 @@
     		.addClass(PageRank.class)
     		.addClass(TriangleListing.class);
     
    +	private ParameterTool parameters;
    --- End diff --
    
    Done.


---
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 #4170: [FLINK-6358] [gelly] Write job details for Gelly e...

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

    https://github.com/apache/flink/pull/4170#discussion_r126313013
  
    --- Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/Runner.java ---
    @@ -103,6 +113,18 @@
     		.addClass(PageRank.class)
     		.addClass(TriangleListing.class);
     
    +	private ParameterTool parameters;
    +
    +	private BooleanParameter disableObjectReuse = new BooleanParameter(this, "__disable_object_reuse");
    +
    +	private StringParameter jobDetails = new StringParameter(this, "__write_job_details")
    --- End diff --
    
    Renamed to `jobDetailsPath` and `__job_details_path`. Plan to add descriptions in the future.


---
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 #4170: [FLINK-6358] [gelly] Write job details for Gelly e...

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

    https://github.com/apache/flink/pull/4170#discussion_r126312866
  
    --- Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/Runner.java ---
    @@ -70,7 +79,8 @@
      * <p>Algorithms must explicitly support each type of output via implementation of
      * interfaces. This is scalable as the number of outputs is small and finite.
      */
    -public class Runner {
    +public class Runner
    +extends ParameterizedBase {
    --- End diff --
    
    `Parameter` / `SimpleParameter` are the interface / base class for parameters whereas `Parameterized` / `ParameterizedBase` are the interface / base class for components containing the parameters.


---
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 #4170: [FLINK-6358] [gelly] Write job details for Gelly e...

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

    https://github.com/apache/flink/pull/4170#discussion_r124551917
  
    --- Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/Runner.java ---
    @@ -103,6 +113,18 @@
     		.addClass(PageRank.class)
     		.addClass(TriangleListing.class);
     
    +	private ParameterTool parameters;
    +
    +	private BooleanParameter disableObjectReuse = new BooleanParameter(this, "__disable_object_reuse");
    +
    +	private StringParameter jobDetails = new StringParameter(this, "__write_job_details")
    --- End diff --
    
    rename parameter to `__job_details_path` ?  `__write_job_details_to_path`?
    
    and rename field to `jobDetailsPath`.
    
    Now it is unclear what this variable represents without looking at the usages.
    



---
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 #4170: [FLINK-6358] [gelly] Write job details for Gelly e...

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

    https://github.com/apache/flink/pull/4170#discussion_r126312914
  
    --- Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/Runner.java ---
    @@ -103,6 +113,18 @@
     		.addClass(PageRank.class)
     		.addClass(TriangleListing.class);
     
    +	private ParameterTool parameters;
    --- End diff --
    
    Done.


---
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 #4170: [FLINK-6358] [gelly] Write job details for Gelly e...

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

    https://github.com/apache/flink/pull/4170#discussion_r126312918
  
    --- Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/Runner.java ---
    @@ -103,6 +113,18 @@
     		.addClass(PageRank.class)
     		.addClass(TriangleListing.class);
     
    +	private ParameterTool parameters;
    +
    +	private BooleanParameter disableObjectReuse = new BooleanParameter(this, "__disable_object_reuse");
    +
    +	private StringParameter jobDetails = new StringParameter(this, "__write_job_details")
    --- End diff --
    
    Done.


---
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 #4170: [FLINK-6358] [gelly] Write job details for Gelly e...

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

    https://github.com/apache/flink/pull/4170#discussion_r124544540
  
    --- Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/Runner.java ---
    @@ -103,6 +113,18 @@
     		.addClass(PageRank.class)
     		.addClass(TriangleListing.class);
     
    +	private ParameterTool parameters;
    +
    +	private BooleanParameter disableObjectReuse = new BooleanParameter(this, "__disable_object_reuse");
    --- End diff --
    
    `private final`?


---
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 #4170: [FLINK-6358] [gelly] Write job details for Gelly e...

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

    https://github.com/apache/flink/pull/4170#discussion_r126313414
  
    --- Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/Runner.java ---
    @@ -305,6 +334,41 @@ public static void main(String[] args) throws Exception {
     			default:
     				throw new ProgramParametrizationException("Unknown output type: " + outputName);
     		}
    +
    +		// ----------------------------------------------------------------------------------------
    +		// Write job details
    --- End diff --
    
    Done.


---
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 #4170: [FLINK-6358] [gelly] Write job details for Gelly e...

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

    https://github.com/apache/flink/pull/4170#discussion_r124544307
  
    --- Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/Runner.java ---
    @@ -103,6 +113,18 @@
     		.addClass(PageRank.class)
     		.addClass(TriangleListing.class);
     
    +	private ParameterTool parameters;
    --- End diff --
    
    set `parameters` in constructor, you will avoid having a null value


---
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 #4170: [FLINK-6358] [gelly] Write job details for Gelly e...

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

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


---
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 #4170: [FLINK-6358] [gelly] Write job details for Gelly e...

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

    https://github.com/apache/flink/pull/4170#discussion_r126312925
  
    --- Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/Runner.java ---
    @@ -103,6 +113,18 @@
     		.addClass(PageRank.class)
     		.addClass(TriangleListing.class);
     
    +	private ParameterTool parameters;
    +
    +	private BooleanParameter disableObjectReuse = new BooleanParameter(this, "__disable_object_reuse");
    +
    +	private StringParameter jobDetails = new StringParameter(this, "__write_job_details")
    +		.setDefaultValue("");
    +
    +	@Override
    +	public String getName() {
    +		return this.getClass().getSimpleName();
    --- End diff --
    
    It is currently unused.


---
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 #4170: [FLINK-6358] [gelly] Write job details for Gelly e...

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

    https://github.com/apache/flink/pull/4170#discussion_r124544513
  
    --- Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/Runner.java ---
    @@ -103,6 +113,18 @@
     		.addClass(PageRank.class)
     		.addClass(TriangleListing.class);
     
    +	private ParameterTool parameters;
    --- End diff --
    
    `private final`?


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