You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by vinodkc <gi...@git.apache.org> on 2017/05/10 01:27:05 UTC
[GitHub] storm pull request #2105: [STORM-2507] Master branch build failure due to ad...
GitHub user vinodkc opened a pull request:
https://github.com/apache/storm/pull/2105
[STORM-2507] Master branch build failure due to additional checkstyle violations in storm-webapp module
Using google-java-format IntelliJ plugin , autoformated storm-webapp module
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/vinodkc/storm-1 br_autoformatstorm-web
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/2105.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 #2105
----
commit b9ab1aeb6eb4bba534643efe5586dd2f7ea61b9f
Author: vinodkc <vi...@gmail.com>
Date: 2017-05-10T01:08:18Z
Using google-java-format IntelliJ plugin , autoformated storm-webapp module to reduce checkstyle issue count
----
---
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 #2105: [STORM-2507] Master branch build failure due to ad...
Posted by vinodkc <gi...@git.apache.org>.
Github user vinodkc commented on a diff in the pull request:
https://github.com/apache/storm/pull/2105#discussion_r115639960
--- Diff: storm-webapp/src/main/java/org/apache/storm/daemon/drpc/DRPCServer.java ---
@@ -45,170 +46,180 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import com.codahale.metrics.Meter;
-import com.google.common.annotations.VisibleForTesting;
-
public class DRPCServer implements AutoCloseable {
- private static final Logger LOG = LoggerFactory.getLogger(DRPCServer.class);
- private final static Meter meterShutdownCalls = StormMetricsRegistry.registerMeter("drpc:num-shutdown-calls");
-
- //TODO in the future this might be better in a common webapp location
- public static void addRequestContextFilter(ServletContextHandler context, String configName, Map<String, Object> conf) {
- IHttpCredentialsPlugin auth = AuthUtils.GetHttpCredentialsPlugin(conf, (String)conf.get(configName));
- ReqContextFilter filter = new ReqContextFilter(auth);
- context.addFilter(new FilterHolder(filter), "/*", FilterMapping.ALL);
- }
-
- private static ThriftServer mkHandlerServer(final DistributedRPC.Iface service, Integer port, Map<String, Object> conf) {
- ThriftServer ret = null;
- if (port != null && port >= 0) {
- ret = new ThriftServer(conf, new DistributedRPC.Processor<>(service),
- ThriftConnectionType.DRPC);
- }
- return ret;
- }
- private static ThriftServer mkInvokeServer(final DistributedRPCInvocations.Iface service, int port, Map<String, Object> conf) {
- return new ThriftServer(conf, new DistributedRPCInvocations.Processor<>(service),
- ThriftConnectionType.DRPC_INVOCATIONS);
- }
-
- private static Server mkHttpServer(Map<String, Object> conf, DRPC drpc) {
- Integer drpcHttpPort = (Integer) conf.get(DaemonConfig.DRPC_HTTP_PORT);
- Server ret = null;
- if (drpcHttpPort != null && drpcHttpPort >= 0) {
- LOG.info("Starting RPC HTTP servers...");
- String filterClass = (String) (conf.get(DaemonConfig.DRPC_HTTP_FILTER));
- @SuppressWarnings("unchecked")
- Map<String, String> filterParams = (Map<String, String>) (conf.get(DaemonConfig.DRPC_HTTP_FILTER_PARAMS));
- FilterConfiguration filterConfiguration = new FilterConfiguration(filterClass, filterParams);
- final List<FilterConfiguration> filterConfigurations = Arrays.asList(filterConfiguration);
- final Integer httpsPort = ObjectReader.getInt(conf.get(DaemonConfig.DRPC_HTTPS_PORT), 0);
- final String httpsKsPath = (String) (conf.get(DaemonConfig.DRPC_HTTPS_KEYSTORE_PATH));
- final String httpsKsPassword = (String) (conf.get(DaemonConfig.DRPC_HTTPS_KEYSTORE_PASSWORD));
- final String httpsKsType = (String) (conf.get(DaemonConfig.DRPC_HTTPS_KEYSTORE_TYPE));
- final String httpsKeyPassword = (String) (conf.get(DaemonConfig.DRPC_HTTPS_KEY_PASSWORD));
- final String httpsTsPath = (String) (conf.get(DaemonConfig.DRPC_HTTPS_TRUSTSTORE_PATH));
- final String httpsTsPassword = (String) (conf.get(DaemonConfig.DRPC_HTTPS_TRUSTSTORE_PASSWORD));
- final String httpsTsType = (String) (conf.get(DaemonConfig.DRPC_HTTPS_TRUSTSTORE_TYPE));
- final Boolean httpsWantClientAuth = (Boolean) (conf.get(DaemonConfig.DRPC_HTTPS_WANT_CLIENT_AUTH));
- final Boolean httpsNeedClientAuth = (Boolean) (conf.get(DaemonConfig.DRPC_HTTPS_NEED_CLIENT_AUTH));
-
- //TODO a better way to do this would be great.
- DRPCApplication.setup(drpc);
- ret = UIHelpers.jettyCreateServer(drpcHttpPort, null, httpsPort);
-
- UIHelpers.configSsl(ret, httpsPort, httpsKsPath, httpsKsPassword, httpsKsType, httpsKeyPassword, httpsTsPath, httpsTsPassword, httpsTsType,
- httpsNeedClientAuth, httpsWantClientAuth);
-
- ServletContextHandler context = new ServletContextHandler(ServletContextHandler.NO_SESSIONS);
- context.setContextPath("/");
- ret.setHandler(context);
-
- ServletHolder jerseyServlet = context.addServlet(ServletContainer.class, "/*");
- jerseyServlet.setInitOrder(1);
- jerseyServlet.setInitParameter("javax.ws.rs.Application", DRPCApplication.class.getName());
-
- UIHelpers.configFilters(context, filterConfigurations);
- addRequestContextFilter(context, Config.DRPC_HTTP_CREDS_PLUGIN, conf);
- }
- return ret;
- }
-
- private final DRPC _drpc;
- private final ThriftServer _handlerServer;
- private final ThriftServer _invokeServer;
- private final Server _httpServer;
- private Thread _handlerServerThread;
- private boolean _closed = false;
-
- public DRPCServer(Map<String, Object> conf) {
- _drpc = new DRPC(conf);
- DRPCThrift thrift = new DRPCThrift(_drpc);
- _handlerServer = mkHandlerServer(thrift, ObjectReader.getInt(conf.get(Config.DRPC_PORT), null), conf);
- _invokeServer = mkInvokeServer(thrift, ObjectReader.getInt(conf.get(Config.DRPC_INVOCATIONS_PORT), 3773), conf);
- _httpServer = mkHttpServer(conf, _drpc);
- }
+ private static final Logger LOG = LoggerFactory.getLogger(DRPCServer.class);
+ private final static Meter meterShutdownCalls = StormMetricsRegistry
+ .registerMeter("drpc:num-shutdown-calls");
+ private final DRPC _drpc;
+ private final ThriftServer _handlerServer;
+ private final ThriftServer _invokeServer;
+ private final Server _httpServer;
+ private Thread _handlerServerThread;
+ private boolean _closed = false;
+ public DRPCServer(Map<String, Object> conf) {
--- End diff --
Recently we have integrated google_checks.xml and because of that checkstyle rules it is formatting code like that. Most of the changes are reordering methods/fields and removal of extra tabs from beginning of line .
Please give your suggestions on whether to add project specific checkstyle rules in storm
---
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 #2105: [STORM-2507] Master branch build failure due to ad...
Posted by vinodkc <gi...@git.apache.org>.
Github user vinodkc closed the pull request at:
https://github.com/apache/storm/pull/2105
---
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 #2105: [STORM-2507] Master branch build failure due to ad...
Posted by erikdw <gi...@git.apache.org>.
Github user erikdw commented on a diff in the pull request:
https://github.com/apache/storm/pull/2105#discussion_r115638968
--- Diff: storm-webapp/src/main/java/org/apache/storm/daemon/drpc/DRPCServer.java ---
@@ -45,170 +46,180 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import com.codahale.metrics.Meter;
-import com.google.common.annotations.VisibleForTesting;
-
public class DRPCServer implements AutoCloseable {
- private static final Logger LOG = LoggerFactory.getLogger(DRPCServer.class);
- private final static Meter meterShutdownCalls = StormMetricsRegistry.registerMeter("drpc:num-shutdown-calls");
-
- //TODO in the future this might be better in a common webapp location
- public static void addRequestContextFilter(ServletContextHandler context, String configName, Map<String, Object> conf) {
- IHttpCredentialsPlugin auth = AuthUtils.GetHttpCredentialsPlugin(conf, (String)conf.get(configName));
- ReqContextFilter filter = new ReqContextFilter(auth);
- context.addFilter(new FilterHolder(filter), "/*", FilterMapping.ALL);
- }
-
- private static ThriftServer mkHandlerServer(final DistributedRPC.Iface service, Integer port, Map<String, Object> conf) {
- ThriftServer ret = null;
- if (port != null && port >= 0) {
- ret = new ThriftServer(conf, new DistributedRPC.Processor<>(service),
- ThriftConnectionType.DRPC);
- }
- return ret;
- }
- private static ThriftServer mkInvokeServer(final DistributedRPCInvocations.Iface service, int port, Map<String, Object> conf) {
- return new ThriftServer(conf, new DistributedRPCInvocations.Processor<>(service),
- ThriftConnectionType.DRPC_INVOCATIONS);
- }
-
- private static Server mkHttpServer(Map<String, Object> conf, DRPC drpc) {
- Integer drpcHttpPort = (Integer) conf.get(DaemonConfig.DRPC_HTTP_PORT);
- Server ret = null;
- if (drpcHttpPort != null && drpcHttpPort >= 0) {
- LOG.info("Starting RPC HTTP servers...");
- String filterClass = (String) (conf.get(DaemonConfig.DRPC_HTTP_FILTER));
- @SuppressWarnings("unchecked")
- Map<String, String> filterParams = (Map<String, String>) (conf.get(DaemonConfig.DRPC_HTTP_FILTER_PARAMS));
- FilterConfiguration filterConfiguration = new FilterConfiguration(filterClass, filterParams);
- final List<FilterConfiguration> filterConfigurations = Arrays.asList(filterConfiguration);
- final Integer httpsPort = ObjectReader.getInt(conf.get(DaemonConfig.DRPC_HTTPS_PORT), 0);
- final String httpsKsPath = (String) (conf.get(DaemonConfig.DRPC_HTTPS_KEYSTORE_PATH));
- final String httpsKsPassword = (String) (conf.get(DaemonConfig.DRPC_HTTPS_KEYSTORE_PASSWORD));
- final String httpsKsType = (String) (conf.get(DaemonConfig.DRPC_HTTPS_KEYSTORE_TYPE));
- final String httpsKeyPassword = (String) (conf.get(DaemonConfig.DRPC_HTTPS_KEY_PASSWORD));
- final String httpsTsPath = (String) (conf.get(DaemonConfig.DRPC_HTTPS_TRUSTSTORE_PATH));
- final String httpsTsPassword = (String) (conf.get(DaemonConfig.DRPC_HTTPS_TRUSTSTORE_PASSWORD));
- final String httpsTsType = (String) (conf.get(DaemonConfig.DRPC_HTTPS_TRUSTSTORE_TYPE));
- final Boolean httpsWantClientAuth = (Boolean) (conf.get(DaemonConfig.DRPC_HTTPS_WANT_CLIENT_AUTH));
- final Boolean httpsNeedClientAuth = (Boolean) (conf.get(DaemonConfig.DRPC_HTTPS_NEED_CLIENT_AUTH));
-
- //TODO a better way to do this would be great.
- DRPCApplication.setup(drpc);
- ret = UIHelpers.jettyCreateServer(drpcHttpPort, null, httpsPort);
-
- UIHelpers.configSsl(ret, httpsPort, httpsKsPath, httpsKsPassword, httpsKsType, httpsKeyPassword, httpsTsPath, httpsTsPassword, httpsTsType,
- httpsNeedClientAuth, httpsWantClientAuth);
-
- ServletContextHandler context = new ServletContextHandler(ServletContextHandler.NO_SESSIONS);
- context.setContextPath("/");
- ret.setHandler(context);
-
- ServletHolder jerseyServlet = context.addServlet(ServletContainer.class, "/*");
- jerseyServlet.setInitOrder(1);
- jerseyServlet.setInitParameter("javax.ws.rs.Application", DRPCApplication.class.getName());
-
- UIHelpers.configFilters(context, filterConfigurations);
- addRequestContextFilter(context, Config.DRPC_HTTP_CREDS_PLUGIN, conf);
- }
- return ret;
- }
-
- private final DRPC _drpc;
- private final ThriftServer _handlerServer;
- private final ThriftServer _invokeServer;
- private final Server _httpServer;
- private Thread _handlerServerThread;
- private boolean _closed = false;
-
- public DRPCServer(Map<String, Object> conf) {
- _drpc = new DRPC(conf);
- DRPCThrift thrift = new DRPCThrift(_drpc);
- _handlerServer = mkHandlerServer(thrift, ObjectReader.getInt(conf.get(Config.DRPC_PORT), null), conf);
- _invokeServer = mkInvokeServer(thrift, ObjectReader.getInt(conf.get(Config.DRPC_INVOCATIONS_PORT), 3773), conf);
- _httpServer = mkHttpServer(conf, _drpc);
- }
+ private static final Logger LOG = LoggerFactory.getLogger(DRPCServer.class);
+ private final static Meter meterShutdownCalls = StormMetricsRegistry
+ .registerMeter("drpc:num-shutdown-calls");
+ private final DRPC _drpc;
+ private final ThriftServer _handlerServer;
+ private final ThriftServer _invokeServer;
+ private final Server _httpServer;
+ private Thread _handlerServerThread;
+ private boolean _closed = false;
+ public DRPCServer(Map<String, Object> conf) {
--- End diff --
This auto-generated formatting this is *SOOOOO* much less readable to me. Is this because of our checkstyle settings? Removing a newline that separates lines is just wrong, and having method calls with their parameters way to the left of the method names is no bueno.
---
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 #2105: [STORM-2507] Master branch build failure due to ad...
Posted by erikdw <gi...@git.apache.org>.
Github user erikdw commented on a diff in the pull request:
https://github.com/apache/storm/pull/2105#discussion_r115647872
--- Diff: storm-webapp/src/main/java/org/apache/storm/daemon/drpc/DRPCServer.java ---
@@ -45,170 +46,180 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import com.codahale.metrics.Meter;
-import com.google.common.annotations.VisibleForTesting;
-
public class DRPCServer implements AutoCloseable {
- private static final Logger LOG = LoggerFactory.getLogger(DRPCServer.class);
- private final static Meter meterShutdownCalls = StormMetricsRegistry.registerMeter("drpc:num-shutdown-calls");
-
- //TODO in the future this might be better in a common webapp location
- public static void addRequestContextFilter(ServletContextHandler context, String configName, Map<String, Object> conf) {
- IHttpCredentialsPlugin auth = AuthUtils.GetHttpCredentialsPlugin(conf, (String)conf.get(configName));
- ReqContextFilter filter = new ReqContextFilter(auth);
- context.addFilter(new FilterHolder(filter), "/*", FilterMapping.ALL);
- }
-
- private static ThriftServer mkHandlerServer(final DistributedRPC.Iface service, Integer port, Map<String, Object> conf) {
- ThriftServer ret = null;
- if (port != null && port >= 0) {
- ret = new ThriftServer(conf, new DistributedRPC.Processor<>(service),
- ThriftConnectionType.DRPC);
- }
- return ret;
- }
- private static ThriftServer mkInvokeServer(final DistributedRPCInvocations.Iface service, int port, Map<String, Object> conf) {
- return new ThriftServer(conf, new DistributedRPCInvocations.Processor<>(service),
- ThriftConnectionType.DRPC_INVOCATIONS);
- }
-
- private static Server mkHttpServer(Map<String, Object> conf, DRPC drpc) {
- Integer drpcHttpPort = (Integer) conf.get(DaemonConfig.DRPC_HTTP_PORT);
- Server ret = null;
- if (drpcHttpPort != null && drpcHttpPort >= 0) {
- LOG.info("Starting RPC HTTP servers...");
- String filterClass = (String) (conf.get(DaemonConfig.DRPC_HTTP_FILTER));
- @SuppressWarnings("unchecked")
- Map<String, String> filterParams = (Map<String, String>) (conf.get(DaemonConfig.DRPC_HTTP_FILTER_PARAMS));
- FilterConfiguration filterConfiguration = new FilterConfiguration(filterClass, filterParams);
- final List<FilterConfiguration> filterConfigurations = Arrays.asList(filterConfiguration);
- final Integer httpsPort = ObjectReader.getInt(conf.get(DaemonConfig.DRPC_HTTPS_PORT), 0);
- final String httpsKsPath = (String) (conf.get(DaemonConfig.DRPC_HTTPS_KEYSTORE_PATH));
- final String httpsKsPassword = (String) (conf.get(DaemonConfig.DRPC_HTTPS_KEYSTORE_PASSWORD));
- final String httpsKsType = (String) (conf.get(DaemonConfig.DRPC_HTTPS_KEYSTORE_TYPE));
- final String httpsKeyPassword = (String) (conf.get(DaemonConfig.DRPC_HTTPS_KEY_PASSWORD));
- final String httpsTsPath = (String) (conf.get(DaemonConfig.DRPC_HTTPS_TRUSTSTORE_PATH));
- final String httpsTsPassword = (String) (conf.get(DaemonConfig.DRPC_HTTPS_TRUSTSTORE_PASSWORD));
- final String httpsTsType = (String) (conf.get(DaemonConfig.DRPC_HTTPS_TRUSTSTORE_TYPE));
- final Boolean httpsWantClientAuth = (Boolean) (conf.get(DaemonConfig.DRPC_HTTPS_WANT_CLIENT_AUTH));
- final Boolean httpsNeedClientAuth = (Boolean) (conf.get(DaemonConfig.DRPC_HTTPS_NEED_CLIENT_AUTH));
-
- //TODO a better way to do this would be great.
- DRPCApplication.setup(drpc);
- ret = UIHelpers.jettyCreateServer(drpcHttpPort, null, httpsPort);
-
- UIHelpers.configSsl(ret, httpsPort, httpsKsPath, httpsKsPassword, httpsKsType, httpsKeyPassword, httpsTsPath, httpsTsPassword, httpsTsType,
- httpsNeedClientAuth, httpsWantClientAuth);
-
- ServletContextHandler context = new ServletContextHandler(ServletContextHandler.NO_SESSIONS);
- context.setContextPath("/");
- ret.setHandler(context);
-
- ServletHolder jerseyServlet = context.addServlet(ServletContainer.class, "/*");
- jerseyServlet.setInitOrder(1);
- jerseyServlet.setInitParameter("javax.ws.rs.Application", DRPCApplication.class.getName());
-
- UIHelpers.configFilters(context, filterConfigurations);
- addRequestContextFilter(context, Config.DRPC_HTTP_CREDS_PLUGIN, conf);
- }
- return ret;
- }
-
- private final DRPC _drpc;
- private final ThriftServer _handlerServer;
- private final ThriftServer _invokeServer;
- private final Server _httpServer;
- private Thread _handlerServerThread;
- private boolean _closed = false;
-
- public DRPCServer(Map<String, Object> conf) {
- _drpc = new DRPC(conf);
- DRPCThrift thrift = new DRPCThrift(_drpc);
- _handlerServer = mkHandlerServer(thrift, ObjectReader.getInt(conf.get(Config.DRPC_PORT), null), conf);
- _invokeServer = mkInvokeServer(thrift, ObjectReader.getInt(conf.get(Config.DRPC_INVOCATIONS_PORT), 3773), conf);
- _httpServer = mkHttpServer(conf, _drpc);
- }
+ private static final Logger LOG = LoggerFactory.getLogger(DRPCServer.class);
+ private final static Meter meterShutdownCalls = StormMetricsRegistry
+ .registerMeter("drpc:num-shutdown-calls");
+ private final DRPC _drpc;
+ private final ThriftServer _handlerServer;
+ private final ThriftServer _invokeServer;
+ private final Server _httpServer;
+ private Thread _handlerServerThread;
+ private boolean _closed = false;
+ public DRPCServer(Map<String, Object> conf) {
--- End diff --
My experience has been that auto-formatting tools all suck. But I'll try to set aside some time to figure out how to make this one suck a little bit less. I really cannot stand this output, I cannot read it.
---
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 #2105: [STORM-2507] Master branch build failure due to additiona...
Posted by vinodkc <gi...@git.apache.org>.
Github user vinodkc commented on the issue:
https://github.com/apache/storm/pull/2105
retest this please
---
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 #2105: [STORM-2507] Master branch build failure due to additiona...
Posted by vinodkc <gi...@git.apache.org>.
Github user vinodkc commented on the issue:
https://github.com/apache/storm/pull/2105
retest this please
---
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.
---