You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@edgent.apache.org by dlaboss <gi...@git.apache.org> on 2016/04/11 22:08:21 UTC
[GitHub] incubator-quarks pull request: [QUARKS-131] [WIP] supply an alias ...
GitHub user dlaboss opened a pull request:
https://github.com/apache/incubator-quarks/pull/86
[QUARKS-131] [WIP] supply an alias for a stream's control services
Looking for feedback for changes to the API.
Impl will follow once this is settled.
Note: relocated PeriodicMXBean as pure Topology/TStream users should not
have to know anything about the quarks.oplet domain.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dlaboss/incubator-quarks quarks-131-controlServiceAlias
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-quarks/pull/86.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 #86
----
commit 3c7877974fbcde5242ae84ee6ff522cbcffa5589
Author: Dale LaBossiere <dl...@us.ibm.com>
Date: 2016-04-11T20:05:33Z
[QUARKS-131] [WIP] supply an alias for a stream's control services
Looking for feedback for changes to the API.
Impl will follow once this is settled.
Note: relocated PeriodicMXBean as pure Topology/TStream users should not
have to know anything about the quarks.oplet domain.
----
---
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-quarks pull request: [QUARKS-131] [REVIEW] supply an ali...
Posted by dlaboss <gi...@git.apache.org>.
Github user dlaboss commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/86#discussion_r60595879
--- Diff: api/graph/src/main/java/quarks/graph/Connector.java ---
@@ -115,5 +115,27 @@ Licensed to the Apache Software Foundation (ASF) under one
*
* @return set of tag values.
*/
- Set<String> getTags();
+ Set<String> getTags();
+
+ /**
+ * Set the alias for the connector.
+ * <p>
+ * The alias must be unique within the topology.
+ * The alias may be used in various contexts:
+ * <ul>
+ * <li>Runtime control services for the connector are registered with this alias.</li>
--- End diff --
Sort of confusing since there are really 3 abstraction levels to the system :-) Would changing "... the connector" => " the Connector" (talking about this class not connectors in the sense of mqtt, iot, ...) or "this Connector" help clarify?
[Maybe I'm just misreading your second sentence. There's nothing stream/TStream specific about a control service nor ControlService. ]
At the API level users deal in terms of TStream.alias() - an alias for the stream and from their perspective control services are registered for the stream.
But at the graph level of the system, its a graph.Connector (the logical equivalent of an output port / stream) that has the alias and services are registered are for the Connector's alias. It has no direct knowledge of TStream.
The oplet level, doesn't have any knowledge of TStream or Connector, it just deals in Consumer for an output port. It needs to register the control service with the alias from the "output port" (aka stream / Connector / TStream). I'm particularly interested in a sanity check on the addition of OutputContext to OpletContext. It, or something else, is needed for the oplet to be able to get the alias to register mbean.
---
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-quarks pull request: [QUARKS-131] [WIP] supply an alias ...
Posted by dlaboss <gi...@git.apache.org>.
Github user dlaboss commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/86#discussion_r59411427
--- Diff: api/execution/src/main/java/quarks/execution/mbeans/PeriodicMXBean.java ---
@@ -16,14 +16,14 @@ Licensed to the Apache Software Foundation (ASF) under one
specific language governing permissions and limitations
under the License.
*/
-package quarks.oplet.core.mbeans;
+package quarks.execution.mbeans;
import java.util.concurrent.TimeUnit;
/**
- * Control interface for a periodic oplet.
+ * Control interface for a periodic source stream.
--- End diff --
makes sense
---
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-quarks pull request: [QUARKS-131] expose control service...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-quarks/pull/86
---
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-quarks pull request: [QUARKS-131] [REVIEW] supply an ali...
Posted by dlaboss <gi...@git.apache.org>.
Github user dlaboss commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/86#discussion_r61105658
--- Diff: api/topology/src/main/java/quarks/topology/TStream.java ---
@@ -443,7 +443,7 @@ else if (WARNING.equals(lr.getLevel()))
Set<String> getTags();
/**
- * Add an alias for the stream.
+ * Set an alias for the stream.
* <p>
* The alias must be unique within the topology.
--- End diff --
https://issues.apache.org/jira/browse/QUARKS-154 is for "type" naming conventions doc/improvements.
---
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-quarks pull request: [QUARKS-131] [REVIEW] supply an ali...
Posted by dlaboss <gi...@git.apache.org>.
Github user dlaboss commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/86#discussion_r61106270
--- Diff: api/oplet/src/main/java/quarks/oplet/core/PeriodicSource.java ---
@@ -47,8 +48,8 @@ public void initialize(OpletContext<Void, T> context) {
public synchronized void start() {
ControlService cs = getOpletContext().getService(ControlService.class);
if (cs != null)
- cs.registerControl("periodic", getOpletContext().uniquify(getClass().getSimpleName()),
- getAlias(), PeriodicMXBean.class, this);
+ cs.registerControl(TStream.TYPE, getOpletContext().uniquify(getClass().getSimpleName()),
--- End diff --
added the above idea to QUARKS-154
---
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-quarks pull request: [QUARKS-131] [REVIEW] supply an ali...
Posted by ddebrunner <gi...@git.apache.org>.
Github user ddebrunner commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/86#discussion_r61005931
--- Diff: api/oplet/src/main/java/quarks/oplet/OutputContext.java ---
@@ -0,0 +1,30 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+package quarks.oplet;
+
+/**
+ * Information about an oplet output port.
+ */
+public interface OutputContext {
--- End diff --
Should this be `OutputPortContext` given its description?
---
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-quarks pull request: [QUARKS-131] expose control service...
Posted by dlaboss <gi...@git.apache.org>.
Github user dlaboss commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/86#discussion_r61290360
--- Diff: api/oplet/src/main/java/quarks/oplet/OpletContext.java ---
@@ -76,6 +76,12 @@ Licensed to the Apache Software Foundation (ASF) under one
List<? extends Consumer<O>> getOutputs();
/**
+ * Get the oplet's output port context information.
+ * @return list of {@link OutputContext}, one for each output port.
+ */
+ List<? extends OutputContext> getOutputContext();
--- End diff --
fixed
---
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-quarks pull request: [QUARKS-131] [REVIEW] supply an ali...
Posted by vdogaru <gi...@git.apache.org>.
Github user vdogaru commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/86#discussion_r61000344
--- Diff: api/graph/src/main/java/quarks/graph/Connector.java ---
@@ -115,5 +115,27 @@ Licensed to the Apache Software Foundation (ASF) under one
*
* @return set of tag values.
*/
- Set<String> getTags();
+ Set<String> getTags();
+
+ /**
+ * Set the alias for the connector.
+ * <p>
+ * The alias must be unique within the topology.
+ * The alias may be used in various contexts:
+ * <ul>
+ * <li>Runtime control services for the connector are registered with this alias.</li>
--- End diff --
> at the graph level ... services are registered for the Connector's alias
Right, Connectors live as long as the Graph, so can serve as registration object.
+1 for OutputContext.
---
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-quarks pull request: [QUARKS-131] [WIP] supply an alias ...
Posted by ddebrunner <gi...@git.apache.org>.
Github user ddebrunner commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/86#discussion_r59408020
--- Diff: api/execution/src/main/java/quarks/execution/mbeans/PeriodicMXBean.java ---
@@ -16,14 +16,14 @@ Licensed to the Apache Software Foundation (ASF) under one
specific language governing permissions and limitations
under the License.
*/
-package quarks.oplet.core.mbeans;
+package quarks.execution.mbeans;
import java.util.concurrent.TimeUnit;
/**
- * Control interface for a periodic oplet.
+ * Control interface for a periodic source stream.
--- End diff --
Since this has moved it now also seems completely generic it could be used for periodic entity, therefore:
`s/source stream/entity/`
?
---
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-quarks pull request: [QUARKS-131] [REVIEW] supply an ali...
Posted by ddebrunner <gi...@git.apache.org>.
Github user ddebrunner commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/86#discussion_r61005884
--- Diff: api/oplet/src/main/java/quarks/oplet/OpletContext.java ---
@@ -76,6 +76,12 @@ Licensed to the Apache Software Foundation (ASF) under one
List<? extends Consumer<O>> getOutputs();
/**
+ * Get the oplet's output port context information.
+ * @return list of {@link OutputContext}, one for each output port.
+ */
+ List<? extends OutputContext> getOutputContext();
--- End diff --
Why `? extends OutputContext` ?
---
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-quarks pull request: [QUARKS-131] [REVIEW] supply an ali...
Posted by vdogaru <gi...@git.apache.org>.
Github user vdogaru commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/86#discussion_r61002673
--- Diff: api/oplet/src/main/java/quarks/oplet/core/PeriodicSource.java ---
@@ -47,8 +48,8 @@ public void initialize(OpletContext<Void, T> context) {
public synchronized void start() {
ControlService cs = getOpletContext().getService(ControlService.class);
if (cs != null)
- cs.registerControl("periodic", getOpletContext().uniquify(getClass().getSimpleName()),
- getAlias(), PeriodicMXBean.class, this);
+ cs.registerControl(TStream.TYPE, getOpletContext().uniquify(getClass().getSimpleName()),
--- End diff --
Would it make sense to register the control against a class type rather than a String? Instead of:
`<T> String registerControl(String type, String id, String alias, Class<T> controlInterface, T control);` where
the `type` parameter identifies the type of the control MBean, use:
`<T> String registerControl(Class<?> type, String id, String alias, Class<T> controlInterface, T control);` where the `type` parameter is the type of object controlled by the MBean.
---
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-quarks pull request: [QUARKS-131] [REVIEW] supply an ali...
Posted by dlaboss <gi...@git.apache.org>.
Github user dlaboss commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/86#discussion_r60499668
--- Diff: api/topology/src/main/java/quarks/topology/TStream.java ---
@@ -443,7 +443,7 @@ else if (WARNING.equals(lr.getLevel()))
Set<String> getTags();
/**
- * Add an alias for the stream.
+ * Set an alias for the stream.
* <p>
* The alias must be unique within the topology.
--- End diff --
I think it has to be that restrictive for things to remain sane. What you say is true, but a TStream.alias() user has no idea or control over what "types" may be registered by the runtime or others using an alias so they can't predict or otherwise avoid collisions, right?
E.g., maybe some day poll() and map() register some mbean with the same type, say, some logging mbean or such?
At a higher level I'm not really getting it wrt picking "type" or alias names, or "ids" for that matter in order to generally avoid collisions since the ControlService is documented as expecting to be used by application code as well as the runtime. Seems like minimally more doc is needed in ControlService for conventions for these. And right now the runtime uses unadorned values like "job" and "periodic" for types - what's to prevent an ignorant user for picking those to use for their own application specific controls?
---
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-quarks pull request: [QUARKS-131] [REVIEW] supply an ali...
Posted by vdogaru <gi...@git.apache.org>.
Github user vdogaru commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/86#discussion_r60471221
--- Diff: api/topology/src/main/java/quarks/topology/TStream.java ---
@@ -443,7 +443,7 @@ else if (WARNING.equals(lr.getLevel()))
Set<String> getTags();
/**
- * Add an alias for the stream.
+ * Set an alias for the stream.
* <p>
* The alias must be unique within the topology.
--- End diff --
Is the statement "must be unique within the topology" too restrictive?
The system registers a control associated with that TStream using ```ControlService.registerControl(String type, String id, String alias, Class<T> controlInterface, T control)```, where the 'alias' parameter is TStream's alias. The registration requires that 'alias' is unique within the context of the control 'type'. So I could have:
```
TStream<Double> t1 = topology.poll(...);
t1.alias("foo");
// At run time, the PeriodicSource oplet registers a PeriodicMXBean with type "periodic" and alias "foo"
...
TStream<String> t2 = t1.map(...);
t2.alias("foo");
// At run time, the Map oplet registers a (hypothetical) MapMXBean with type "map" and alias "foo"
```
---
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-quarks pull request: [QUARKS-131] [REVIEW] supply an ali...
Posted by vdogaru <gi...@git.apache.org>.
Github user vdogaru commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/86#discussion_r61001209
--- Diff: api/topology/src/main/java/quarks/topology/TStream.java ---
@@ -443,7 +443,7 @@ else if (WARNING.equals(lr.getLevel()))
Set<String> getTags();
/**
- * Add an alias for the stream.
+ * Set an alias for the stream.
* <p>
* The alias must be unique within the topology.
--- End diff --
Maybe we need a naming convention similar to the proposal for tag values - https://issues.apache.org/jira/browse/QUARKS-68 ?
---
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-quarks pull request: [QUARKS-131] [REVIEW] supply an ali...
Posted by vdogaru <gi...@git.apache.org>.
Github user vdogaru commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/86#discussion_r60467472
--- Diff: api/graph/src/main/java/quarks/graph/Connector.java ---
@@ -115,5 +115,27 @@ Licensed to the Apache Software Foundation (ASF) under one
*
* @return set of tag values.
*/
- Set<String> getTags();
+ Set<String> getTags();
+
+ /**
+ * Set the alias for the connector.
+ * <p>
+ * The alias must be unique within the topology.
+ * The alias may be used in various contexts:
+ * <ul>
+ * <li>Runtime control services for the connector are registered with this alias.</li>
--- End diff --
I'm a bit confused by "Runtime control services for the connector". At the high level, a control service provides an interface for controlling the stream and is registered with the TStream's alias.
---
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-quarks pull request: [QUARKS-131] [REVIEW] supply an ali...
Posted by dlaboss <gi...@git.apache.org>.
Github user dlaboss commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/86#discussion_r61089125
--- Diff: api/oplet/src/main/java/quarks/oplet/OutputContext.java ---
@@ -0,0 +1,30 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+package quarks.oplet;
+
+/**
+ * Information about an oplet output port.
+ */
+public interface OutputContext {
--- End diff --
Sure, works for me! I initially omitted "port" because the oplet layer seems to omit it - e.g., ``OpletContext.getOutputs()``.
---
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.
---