[GitHub] qpid-proton pull request: PROTON-984: Document proton-j time units
Github user boo closed the pull request at: https://github.com/apache/qpid-proton/pull/50 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] qpid-proton pull request: Proton 980
Github user boo commented on the pull request: https://github.com/apache/qpid-proton/pull/49#issuecomment-139454327 rebased to master --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] qpid-proton pull request: Proton 980
Github user boo closed the pull request at: https://github.com/apache/qpid-proton/pull/49 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] qpid-proton pull request: PROTON-984: Document proton-j time units
GitHub user boo opened a pull request: https://github.com/apache/qpid-proton/pull/50 PROTON-984: Document proton-j time units Can somebody help me fill out the blanks here? I think proton-j uses milliseconds throughout, and what does Reactor#setTimeout() do? You can merge this pull request into a Git repository by running: $ git pull https://github.com/boo/qpid-proton proton-984 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/qpid-proton/pull/50.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 #50 commit 60d630d63b41b5ba665dcd093c5927a89257a7e9 Author: Bozo Dragojevic bo...@digiverse.si Date: 2015-07-22T08:41:30Z PROTON-984: Document proton-j time units --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] qpid-proton pull request: Proton 980
GitHub user boo opened a pull request: https://github.com/apache/qpid-proton/pull/49 Proton 980 You can merge this pull request into a Git repository by running: $ git pull https://github.com/boo/qpid-proton proton-980 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/qpid-proton/pull/49.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 #49 commit 35cac00dae28caa8e9d972957f76586489904743 Author: Bozo Dragojevic bo...@digiverse.si Date: 2015-07-28T13:50:02Z NO-JIRA: Handlers in a HashedSet seems fragile Add a test for dispatching of event through child handlers commit d1ec5903629364552f721f3f57b246017ebce60e Author: Bozo Dragojevic bo...@digiverse.si Date: 2015-07-29T11:52:23Z PROTON-980: Enable handler processing the event after child handlers have processed 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] qpid-proton pull request: PROTON-964: Proton-J extensible event ty...
Github user boo commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/48#discussion_r36522686
--- Diff:
proton-j/src/main/java/org/apache/qpid/proton/engine/impl/CollectorImpl.java ---
@@ -57,9 +58,16 @@ public void pop()
}
}
-public EventImpl put(Event.Type type, Object context)
+public EventImpl put(EventType type, Object context)
{
-if (tail != null tail.getType() == type
+if (type == null) {
+throw new IllegalArgumentException(Type cannot be null);
+}
+if (!type.isValid()) {
+throw new IllegalArgumentException(Cannot put events of type
+ type);
+}
+// XXX: @gemmellr points out that type equality is wrong for event
de-duplication. What about context equality?
--- End diff --
I can respin or drop the commit :)
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] qpid-proton pull request: PROTON-964: Proton-J extensible event ty...
Github user boo commented on the pull request: https://github.com/apache/qpid-proton/pull/48#issuecomment-128358421 @gemmellr I think I have managed to eliminate almost all casts by delegating half of the dispatch to the handler -- the handler already knows it's type, no need for generics even. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] qpid-proton pull request: PROTON-964: Proton-J extensible event ty...
Github user boo commented on a diff in the pull request: https://github.com/apache/qpid-proton/pull/48#discussion_r36181146 --- Diff: proton-j/src/main/java/org/apache/qpid/proton/engine/impl/EventImpl.java --- @@ -217,6 +101,19 @@ public void dispatch(Handler handler) dispatch(children.next()); } } + +@Override +public void redispatch(EventType as_type, Handler handler) throws HandlerException --- End diff -- This is for now the only way to actually generate non-core, events... I'll extend the javadoc to make it explicit. This was the minimal change needed and right now behaves as you describe. A typical example would be a message decoder handler that turns a DELIVERY event into a MESSAGE event with attached message in the event. Or a timer for periodic sampling of a Connection, and so on. I'm in fact sitting on some more reactor/collector changes that were done later and for other use cases. One of them is the ability for a handler to have children process the event synchronously. I can also fold the implementation of that change in here as it would then allow redispatch() to have child handlers be invoked first with the original event, before redispatching the new type. And keep exposing the interface for that in a separate jira. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] qpid-proton pull request: PROTON-964: Proton-J extensible event ty...
Github user boo commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/48#discussion_r36182863
--- Diff:
proton-j/src/main/java/org/apache/qpid/proton/engine/impl/CollectorImpl.java ---
@@ -57,9 +58,13 @@ public void pop()
}
}
-public EventImpl put(Event.Type type, Object context)
+public EventImpl put(EventType type, Object context)
{
-if (tail != null tail.getType() == type
+if (type == null)
+throw new IllegalArgumentException(Type cannot be null);
+if (!type.isValid())
+throw new IllegalArgumentException(Cannot put events of type
+ type);
+if (tail != null tail.getEventType() == type
--- End diff --
Can you please qualify 'the wrong thing', I really want to understand this?
With this overall change, I'm only working around the `final` of the enum,
not around other aspects of it's semantics. This change brings us about on-par
with proton-c imho where EventType === int.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] qpid-proton pull request: PROTON-964: Proton-J extensible event ty...
Github user boo commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/48#discussion_r36180019
--- Diff:
proton-j/src/main/java/org/apache/qpid/proton/engine/impl/EventImpl.java ---
@@ -53,9 +54,9 @@
this.type = null;
}
-void init(Event.Type type, Object context)
+void init(EventType type2, Object context)
{
-this.type = type;
+this.type = type2;
--- End diff --
uhuh, will fix
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] qpid-proton pull request: PROTON-964: Proton-J extensible event ty...
Github user boo commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/48#discussion_r36178995
--- Diff: proton-j/src/main/java/org/apache/qpid/proton/engine/Event.java
---
@@ -80,15 +83,46 @@
SELECTABLE_WRITABLE,
SELECTABLE_EXPIRED,
SELECTABLE_ERROR,
-SELECTABLE_FINAL
+SELECTABLE_FINAL,
+
+/**
+ * This value must never be used to generate an event, it's only
used as
+ * a guard when casting custom EventTypes to builtin {@link Type}
via
+ * {@link Event#getBuiltinType()}.
+ */
+NOT_A_BUILTIN_TYPE;
--- End diff --
I'll go with something on the theme NON_CORE_EVENT?
The getBuiltinType() is from a previous iteration of the patch and should
have been renamed to getType()
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] qpid-proton pull request: PROTON-964: Proton-J extensible event ty...
Github user boo commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/48#discussion_r36179005
--- Diff: proton-j/src/main/java/org/apache/qpid/proton/engine/Record.java
---
@@ -29,6 +29,10 @@
public interface Record
{
+public interface AccessorT {
--- End diff --
Ack
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] qpid-proton pull request: PROTON-964: Proton-J extensible event ty...
Github user boo commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/48#discussion_r36179169
--- Diff:
proton-j/src/main/java/org/apache/qpid/proton/engine/impl/BuiltinEventTypeImpl.java
---
@@ -0,0 +1,144 @@
+package org.apache.qpid.proton.engine.impl;
+
+import org.apache.qpid.proton.engine.BuiltinHandler;
+import org.apache.qpid.proton.engine.Event;
+import org.apache.qpid.proton.engine.Handler;
+
+public class BuiltinEventTypeImpl {
--- End diff --
This is out-of-line implementation of EventType interface for
Event.Type#dispatch.
CoreEventDispatcher?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] qpid-proton pull request: PROTON-964: Proton-J extensible event ty...
Github user boo commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/48#discussion_r36179236
--- Diff:
proton-j/src/main/java/org/apache/qpid/proton/engine/impl/CollectorImpl.java ---
@@ -57,9 +58,13 @@ public void pop()
}
}
-public EventImpl put(Event.Type type, Object context)
+public EventImpl put(EventType type, Object context)
{
-if (tail != null tail.getType() == type
+if (type == null)
--- End diff --
ack
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] qpid-proton pull request: PROTON-964: Proton-J extensible event ty...
Github user boo commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/48#discussion_r36179971
--- Diff:
proton-j/src/main/java/org/apache/qpid/proton/engine/impl/CollectorImpl.java ---
@@ -57,9 +58,13 @@ public void pop()
}
}
-public EventImpl put(Event.Type type, Object context)
+public EventImpl put(EventType type, Object context)
{
-if (tail != null tail.getType() == type
+if (type == null)
+throw new IllegalArgumentException(Type cannot be null);
+if (!type.isValid())
+throw new IllegalArgumentException(Cannot put events of type
+ type);
+if (tail != null tail.getEventType() == type
--- End diff --
Frankly, I'm not even sure why this check is needed at all.
But extensibe events don't need to change collector semantics. the change
here is
a mostly about robustness and a bit about the fact that not only core
events are now possible.
If needed this should be a separate change with separate justification.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] qpid-proton pull request: PROTON-964: Proton-J extensible event ty...
GitHub user boo opened a pull request: https://github.com/apache/qpid-proton/pull/48 PROTON-964: Proton-J extensible event types You can merge this pull request into a Git repository by running: $ git pull https://github.com/boo/qpid-proton proton-964 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/qpid-proton/pull/48.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 #48 commit a72c23a136dddae7efd6385d3b32c406c861a016 Author: Bozo Dragojevic bo...@digiverse.si Date: 2015-07-13T10:57:13Z PROTON-964: Proton-J extensible event types --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] qpid-proton pull request: PROTON-838: proton-hawtdispatch cannot c...
GitHub user boo opened a pull request: https://github.com/apache/qpid-proton/pull/12 PROTON-838: proton-hawtdispatch cannot connect with SSL Remove one of calls to transport.connecting() as this causes an assert with SslTransport You can merge this pull request into a Git repository by running: $ git pull https://github.com/boo/qpid-proton master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/qpid-proton/pull/12.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 #12 commit 0d2b47be126ad7619e1c926f88a69297afebbba9 Author: Bozo Dragojevic bo...@digiverse.si Date: 2015-03-13T13:01:35Z PROTON-838: proton-hawtdispatch cannot connect with SSL Remove one of calls to transport.connecting() as this causes an assert with SslTransport --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
