You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metron.apache.org by cestella <gi...@git.apache.org> on 2016/08/24 14:18:29 UTC
[GitHub] incubator-metron pull request #228: METRON-390: Stellar functions should ini...
GitHub user cestella opened a pull request:
https://github.com/apache/incubator-metron/pull/228
METRON-390: Stellar functions should initialize upon use, rather than all at once
To test this, you should just spin up the environment in vagrant and everything should work as expected.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cestella/incubator-metron stellar_initialization
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-metron/pull/228.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 #228
----
commit f2860ce8bd744df8db100fe96719163362c07294
Author: cstella <ce...@gmail.com>
Date: 2016-08-24T14:14:28Z
METRON-390: Stellar functions should initialize upon use, rather than all at once
----
---
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] incubator-metron pull request #228: METRON-390: Stellar functions should ini...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-metron/pull/228
---
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] incubator-metron pull request #228: METRON-390: Stellar functions should ini...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/228#discussion_r76096910
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/dsl/functions/MaaSFunctions.java ---
@@ -234,25 +248,36 @@ public Object apply(List<Object> args, Context context) throws ParseException {
}
return ret;
}
+
@Override
- public void initialize(Context context) {
- Optional<Object> clientOptional = context.getCapability(Context.Capabilities.ZOOKEEPER_CLIENT);
- CuratorFramework client = null;
- if(clientOptional.isPresent() && clientOptional.get() instanceof CuratorFramework) {
- client = (CuratorFramework)clientOptional.get();
- }
- else {
- return;
- }
+ public synchronized void initialize(Context context) {
try {
- MaaSConfig config = ConfigUtil.INSTANCE.read(client, "/metron/maas/config", new MaaSConfig(), MaaSConfig.class);
- discoverer = new ServiceDiscoverer(client, config.getServiceRoot());
- discoverer.start();
- context.addCapability(Context.Capabilities.SERVICE_DISCOVERER, () -> discoverer);
- } catch (Exception e) {
- LOG.error(e.getMessage(), e);
- return;
+ Optional<Object> clientOptional = context.getCapability(Context.Capabilities.ZOOKEEPER_CLIENT);
+ CuratorFramework client = null;
+ if (clientOptional.isPresent() && clientOptional.get() instanceof CuratorFramework) {
+ client = (CuratorFramework) clientOptional.get();
+ } else {
+ return;
+ }
+ try {
+ MaaSConfig config = ConfigUtil.INSTANCE.read(client, "/metron/maas/config", new MaaSConfig(), MaaSConfig.class);
+ discoverer = new ServiceDiscoverer(client, config.getServiceRoot());
+ discoverer.start();
+ context.addCapability(Context.Capabilities.SERVICE_DISCOVERER, () -> discoverer);
+ isValidState = true;
+ } catch (Exception e) {
+ LOG.error(e.getMessage(), e);
--- End diff --
If initialization fails, we're just logging the error. Does it not make sense to fail fast or make some noise?
---
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] incubator-metron issue #228: METRON-390: Stellar functions should initialize...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/incubator-metron/pull/228
+1 looks good
---
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] incubator-metron pull request #228: METRON-390: Stellar functions should ini...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/228#discussion_r76096625
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/dsl/functions/MaaSFunctions.java ---
@@ -188,15 +189,27 @@ public void initialize(Context context) {
catch(Exception ex) {
LOG.error(ex.getMessage(), ex);
}
+ finally {
+ isInitialized = true;
--- End diff --
If initialization fails, do we will want to set `isInitialized` to true?
---
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] incubator-metron pull request #228: METRON-390: Stellar functions should ini...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/228#discussion_r76099089
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/dsl/functions/MaaSFunctions.java ---
@@ -188,15 +189,27 @@ public void initialize(Context context) {
catch(Exception ex) {
LOG.error(ex.getMessage(), ex);
}
+ finally {
+ isInitialized = true;
--- End diff --
That makes sense. Maybe a little comment to that effect, will prevent me from asking you this same question in a few months. Ha.
---
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] incubator-metron pull request #228: METRON-390: Stellar functions should ini...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/228#discussion_r76098304
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/dsl/functions/MaaSFunctions.java ---
@@ -234,25 +248,36 @@ public Object apply(List<Object> args, Context context) throws ParseException {
}
return ret;
}
+
@Override
- public void initialize(Context context) {
- Optional<Object> clientOptional = context.getCapability(Context.Capabilities.ZOOKEEPER_CLIENT);
- CuratorFramework client = null;
- if(clientOptional.isPresent() && clientOptional.get() instanceof CuratorFramework) {
- client = (CuratorFramework)clientOptional.get();
- }
- else {
- return;
- }
+ public synchronized void initialize(Context context) {
try {
- MaaSConfig config = ConfigUtil.INSTANCE.read(client, "/metron/maas/config", new MaaSConfig(), MaaSConfig.class);
- discoverer = new ServiceDiscoverer(client, config.getServiceRoot());
- discoverer.start();
- context.addCapability(Context.Capabilities.SERVICE_DISCOVERER, () -> discoverer);
- } catch (Exception e) {
- LOG.error(e.getMessage(), e);
- return;
+ Optional<Object> clientOptional = context.getCapability(Context.Capabilities.ZOOKEEPER_CLIENT);
+ CuratorFramework client = null;
+ if (clientOptional.isPresent() && clientOptional.get() instanceof CuratorFramework) {
+ client = (CuratorFramework) clientOptional.get();
+ } else {
+ return;
+ }
+ try {
+ MaaSConfig config = ConfigUtil.INSTANCE.read(client, "/metron/maas/config", new MaaSConfig(), MaaSConfig.class);
+ discoverer = new ServiceDiscoverer(client, config.getServiceRoot());
+ discoverer.start();
+ context.addCapability(Context.Capabilities.SERVICE_DISCOVERER, () -> discoverer);
+ isValidState = true;
+ } catch (Exception e) {
+ LOG.error(e.getMessage(), e);
--- End diff --
Yeah, agreed. Throwing an exception here.
---
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] incubator-metron pull request #228: METRON-390: Stellar functions should ini...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/228#discussion_r76098276
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/dsl/functions/MaaSFunctions.java ---
@@ -188,15 +189,27 @@ public void initialize(Context context) {
catch(Exception ex) {
LOG.error(ex.getMessage(), ex);
}
+ finally {
+ isInitialized = true;
--- End diff --
I'd prefer for initialization to not continue to retry for every message, but rather the initialization routine should throw an exception.
---
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.
---