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 2017/05/25 22:18:18 UTC

[GitHub] flink pull request #3990: [FLINK-6701] Activate strict checkstyle for flink-...

GitHub user zentol opened a pull request:

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

    [FLINK-6701] Activate strict checkstyle for flink-yarn

    

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

    $ git pull https://github.com/zentol/flink 6701_checkstyle_yarn

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

    https://github.com/apache/flink/pull/3990.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 #3990
    
----
commit 773674f166677dbade3eb6856514a243006e2966
Author: zentol <ch...@apache.org>
Date:   2017-05-24T13:10:15Z

    [FLINK-6701] Activate strict checkstyle for flink-yarn

----


---
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 #3990: [FLINK-6701] Activate strict checkstyle for flink-...

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

    https://github.com/apache/flink/pull/3990#discussion_r118719900
  
    --- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/AbstractYarnClusterDescriptor.java ---
    @@ -984,11 +983,10 @@ private void failSessionDuringDeployment(YarnClient yarnClient, YarnClientApplic
     		yarnClient.stop();
     	}
     
    -
     	private static class ClusterResourceDescription {
    -		final public int totalFreeMemory;
    -		final public int containerLimit;
    -		final public int[] nodeManagersFree;
    +	public final int totalFreeMemory;
    +	public final int containerLimit;
    +	public final int[] nodeManagersFree;
    --- End diff --
    
    Indentation.


---
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 #3990: [FLINK-6701] Activate strict checkstyle for flink-...

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

    https://github.com/apache/flink/pull/3990#discussion_r118723341
  
    --- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/YarnResourceManager.java ---
    @@ -37,75 +37,74 @@
     import org.apache.flink.runtime.resourcemanager.slotmanager.SlotManager;
     import org.apache.flink.runtime.rpc.FatalErrorHandler;
     import org.apache.flink.runtime.rpc.RpcService;
    +import org.apache.flink.util.ExceptionUtils;
    +
     import org.apache.hadoop.yarn.api.ApplicationConstants;
    -import org.apache.hadoop.yarn.api.records.FinalApplicationStatus;
    -import org.apache.hadoop.yarn.api.records.Priority;
    -import org.apache.hadoop.yarn.api.records.Resource;
    -import org.apache.hadoop.yarn.api.records.ContainerStatus;
     import org.apache.hadoop.yarn.api.records.Container;
     import org.apache.hadoop.yarn.api.records.ContainerLaunchContext;
    +import org.apache.hadoop.yarn.api.records.ContainerStatus;
    +import org.apache.hadoop.yarn.api.records.FinalApplicationStatus;
     import org.apache.hadoop.yarn.api.records.NodeReport;
    +import org.apache.hadoop.yarn.api.records.Priority;
    +import org.apache.hadoop.yarn.api.records.Resource;
     import org.apache.hadoop.yarn.client.api.AMRMClient;
     import org.apache.hadoop.yarn.client.api.NMClient;
     import org.apache.hadoop.yarn.client.api.async.AMRMClientAsync;
     import org.apache.hadoop.yarn.conf.YarnConfiguration;
    -import org.slf4j.Logger;
    -import org.slf4j.LoggerFactory;
    -import scala.concurrent.duration.FiniteDuration;
    -import org.apache.flink.util.ExceptionUtils;
     
    -import java.util.Map;
     import java.util.HashMap;
     import java.util.List;
    +import java.util.Map;
     import java.util.concurrent.TimeUnit;
     
    +import scala.concurrent.duration.FiniteDuration;
    +
     /**
      * The yarn implementation of the resource manager. Used when the system is started
      * via the resource framework YARN.
      */
     public class YarnResourceManager extends ResourceManager<ResourceID> implements AMRMClientAsync.CallbackHandler {
    -	protected final Logger LOG = LoggerFactory.getLogger(getClass());
    --- End diff --
    
    Is it intended that the logger be inherited from `RpcEndpoint`?


---
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 #3990: [FLINK-6701] Activate strict checkstyle for flink-yarn

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

    https://github.com/apache/flink/pull/3990
  
    @greghogan I've rebased the branch and addressed your comments.


---
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 #3990: [FLINK-6701] Activate strict checkstyle for flink-...

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

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


---
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 #3990: [FLINK-6701] Activate strict checkstyle for flink-...

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

    https://github.com/apache/flink/pull/3990#discussion_r118720449
  
    --- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/YarnApplicationMasterRunner.java ---
    @@ -494,10 +485,10 @@ protected int runApplicationMaster(Configuration config) {
     	// ------------------------------------------------------------------------
     
     	/**
    -	 * 
    +	 *
    --- End diff --
    
    Incomplete documentation.


---
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 #3990: [FLINK-6701] Activate strict checkstyle for flink-yarn

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

    https://github.com/apache/flink/pull/3990
  
    +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] flink pull request #3990: [FLINK-6701] Activate strict checkstyle for flink-...

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

    https://github.com/apache/flink/pull/3990#discussion_r118726140
  
    --- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/YarnResourceManager.java ---
    @@ -37,75 +37,74 @@
     import org.apache.flink.runtime.resourcemanager.slotmanager.SlotManager;
     import org.apache.flink.runtime.rpc.FatalErrorHandler;
     import org.apache.flink.runtime.rpc.RpcService;
    +import org.apache.flink.util.ExceptionUtils;
    +
     import org.apache.hadoop.yarn.api.ApplicationConstants;
    -import org.apache.hadoop.yarn.api.records.FinalApplicationStatus;
    -import org.apache.hadoop.yarn.api.records.Priority;
    -import org.apache.hadoop.yarn.api.records.Resource;
    -import org.apache.hadoop.yarn.api.records.ContainerStatus;
     import org.apache.hadoop.yarn.api.records.Container;
     import org.apache.hadoop.yarn.api.records.ContainerLaunchContext;
    +import org.apache.hadoop.yarn.api.records.ContainerStatus;
    +import org.apache.hadoop.yarn.api.records.FinalApplicationStatus;
     import org.apache.hadoop.yarn.api.records.NodeReport;
    +import org.apache.hadoop.yarn.api.records.Priority;
    +import org.apache.hadoop.yarn.api.records.Resource;
     import org.apache.hadoop.yarn.client.api.AMRMClient;
     import org.apache.hadoop.yarn.client.api.NMClient;
     import org.apache.hadoop.yarn.client.api.async.AMRMClientAsync;
     import org.apache.hadoop.yarn.conf.YarnConfiguration;
    -import org.slf4j.Logger;
    -import org.slf4j.LoggerFactory;
    -import scala.concurrent.duration.FiniteDuration;
    -import org.apache.flink.util.ExceptionUtils;
     
    -import java.util.Map;
     import java.util.HashMap;
     import java.util.List;
    +import java.util.Map;
     import java.util.concurrent.TimeUnit;
     
    +import scala.concurrent.duration.FiniteDuration;
    +
     /**
      * The yarn implementation of the resource manager. Used when the system is started
      * via the resource framework YARN.
      */
     public class YarnResourceManager extends ResourceManager<ResourceID> implements AMRMClientAsync.CallbackHandler {
    -	protected final Logger LOG = LoggerFactory.getLogger(getClass());
    --- End diff --
    
    Yes. They both call `getClass()` and thus should look the same from the outside.


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