[jira] [Commented] (SLING-3223) Donation of a replication module for Sling
[ https://issues.apache.org/jira/browse/SLING-3223?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13837456#comment-13837456 ] Tommaso Teofili commented on SLING-3223: the IP Clearance has been done (SLING-3254) Donation of a replication module for Sling -- Key: SLING-3223 URL: https://issues.apache.org/jira/browse/SLING-3223 Project: Sling Issue Type: Task Components: Extensions Reporter: Tommaso Teofili Attachments: SLING-3223.patch Issue to track donation of a replication module for Sling. Discussion thread: http://markmail.org/thread/ic62k5pc34ppb5ko Vote on dev@sling thread: http://markmail.org/thread/scz5arlfs5rgowg2 Result vote on dev@sling: http://markmail.org/thread/ix7s4fzvsdwmxrpk -- This message was sent by Atlassian JIRA (v6.1#6144)
[jira] [Commented] (SLING-3223) Donation of a replication module for Sling
[ https://issues.apache.org/jira/browse/SLING-3223?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13819979#comment-13819979 ] Bertrand Delacretaz commented on SLING-3223: Here's a digest of the patch that we should use for the votes and IP clearance that's related to it: SHA1(SLING-3223.patch.txt)= ee628f4556c71c19fa09ae4e58fc7b32182da11b Donation of a replication module for Sling -- Key: SLING-3223 URL: https://issues.apache.org/jira/browse/SLING-3223 Project: Sling Issue Type: Task Components: Extensions Reporter: Tommaso Teofili Attachments: SLING-3223.patch Issue to track donation of a replication module for Sling, see thread at http://markmail.org/thread/ic62k5pc34ppb5ko -- This message was sent by Atlassian JIRA (v6.1#6144)
[jira] [Commented] (SLING-3223) Donation of a replication module for Sling
[ https://issues.apache.org/jira/browse/SLING-3223?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13819988#comment-13819988 ] Bertrand Delacretaz commented on SLING-3223: I generated a test coverage report (by using our 19-SNAPSHOT and building with -P jacoco-report) and test coverage is fairly low, 29% globally and it looks like it's mostly happy cases that are tested. IMO a replication module deserves much better coverage, as troubleshooting it can be quite costly. Donation of a replication module for Sling -- Key: SLING-3223 URL: https://issues.apache.org/jira/browse/SLING-3223 Project: Sling Issue Type: Task Components: Extensions Reporter: Tommaso Teofili Attachments: SLING-3223.patch Issue to track donation of a replication module for Sling, see thread at http://markmail.org/thread/ic62k5pc34ppb5ko -- This message was sent by Atlassian JIRA (v6.1#6144)
[jira] [Commented] (SLING-3223) Donation of a replication module for Sling
[ https://issues.apache.org/jira/browse/SLING-3223?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13819997#comment-13819997 ] Tommaso Teofili commented on SLING-3223: Thanks Bertrand for looking into it, I agree that needs to be fixed (also integration tests are needed IMO), I'll start working on that right now. Donation of a replication module for Sling -- Key: SLING-3223 URL: https://issues.apache.org/jira/browse/SLING-3223 Project: Sling Issue Type: Task Components: Extensions Reporter: Tommaso Teofili Attachments: SLING-3223.patch Issue to track donation of a replication module for Sling, see thread at http://markmail.org/thread/ic62k5pc34ppb5ko -- This message was sent by Atlassian JIRA (v6.1#6144)
[jira] [Commented] (SLING-3223) Donation of a replication module for Sling
[ https://issues.apache.org/jira/browse/SLING-3223?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13817198#comment-13817198 ] Tommaso Teofili commented on SLING-3223: bq. but then, is there really a difference between activate and deacticate ? the difference is just in the package is deserialized which is up to the ReplicationPackageBuilder (which takes care of both serialization and deserialization) bq. ok. consider using a different name, then good suggestion bq. I would get rid of it and put it into an utility class, or into the servlet where you need it. it just pollutes the set of adapters in sling. I see your point, I wouldn't put it into the servlet, perhaps a utility class works good. thanks Tobias. I there's no other concern I'd start the vote on dev@ next week. Donation of a replication module for Sling -- Key: SLING-3223 URL: https://issues.apache.org/jira/browse/SLING-3223 Project: Sling Issue Type: Task Components: Extensions Reporter: Tommaso Teofili Attachments: SLING-3223.patch Issue to track donation of a replication module for Sling, see thread at http://markmail.org/thread/ic62k5pc34ppb5ko -- This message was sent by Atlassian JIRA (v6.1#6144)
[jira] [Commented] (SLING-3223) Donation of a replication module for Sling
[ https://issues.apache.org/jira/browse/SLING-3223?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13814731#comment-13814731 ] Tommaso Teofili commented on SLING-3223: thanks for your comments Tobias. bq, way to little documentation (javadoc, general architecture) yes, while I have some doc written in a README.md, javadoc needs to be improved. bq. Don't use HTTP headers to transport additional information. this makes it harder to get request correctly though proxies and firewalls ... If you use customer HTTP headers, it's best practice to prepend an app specific prefix, eg: X-Sling-Replication-Action good points, then trying to get rid of the headers at all sounds like the best option bq why do you expose a ReplicationConfigurationServlet and not rely on default mechanisms to update config, e.g. update the config via SlingPostServlet or via felix console? (Is there some functionality missing in Sling or does every component needs to provide it's own management REST API?) it's not something missing, I'd like to provide an HTTP/REST API for working with agents, that including triggering replication, checking/updating configuration, checking queues (TBD), creating/deleting agent (TBD). If there's a smarter way of doing such things you may think of just let me know. Current HTTP API has: HTTP POST (/w parameters) to _http://host:port/system/replication/agents_ for triggering replication of all existing agents HTTP POST (/w parameters) to _http://host:port/system/replication/agent/foo_ for triggering replication of agent foo HTTP GET _http://host:port/system/replication/agent/configuration/foo_ for retrieving configuration of agent foo HTTP POST (/w parameters) to _http://host:port/system/replication/agent/configuration/foo_ for updating configuration of agent foo bq. I would reconsider if Activate and Deactivate really are good names. sure, I agree add and delete sound more appropriate. bq. btw: how is deactivate implemented? not via an empty content package? an empty package (see _VoidReplicationPackage_) bq. I don't know if TriggerPathReplicationRule is really a sufficient filter. Usually it's hard to determined at what point a modified subtree is ready to replicate. you would probably need to add much more logic to it right, that's hard to do in general if e.g. create operations are not atomic (create /a/b, persist, create /a/b/c persist, create /a/b/c/d etc.) so depending on the use case something like collecting bunch of changes may work better; however that's good you raised the point as that will need to be handled properly. bq. I don't quite understand AuthenticationHandler. Are those providers that are used to add authentication to the HTTP requests, or are they consumers that are used to authenticate replication payloads? it's the former, but it's not just for HTTP, they're used for authenticate _TransportHandlers_ which may deliver content to either an HTTP endpoint (most often) or not (e.g. file system, or other) bq. the adapter from SlingRequest to ReplicationPackage is really questionable in IMO. It is used in a very specific case and the adaption is considerably expensive. Or do you foresee that other clients of this API adapt a request to a ReplicationPackage ? I agree that can be much improved, that's just used from the receiving servlet for reading a replication package stream to install the package on the receiving instance. bq. suggest to add package-info.java files with the bundle export information over Export-Package elements in the pom.xml agreed while collecting other feedback (if any) I'll work on the topics brought by Tobias. Donation of a replication module for Sling -- Key: SLING-3223 URL: https://issues.apache.org/jira/browse/SLING-3223 Project: Sling Issue Type: Task Components: Extensions Reporter: Tommaso Teofili Attachments: SLING-3223.patch Issue to track donation of a replication module for Sling, see thread at http://markmail.org/thread/ic62k5pc34ppb5ko -- This message was sent by Atlassian JIRA (v6.1#6144)
[jira] [Commented] (SLING-3223) Donation of a replication module for Sling
[
https://issues.apache.org/jira/browse/SLING-3223?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13814738#comment-13814738
]
Bertrand Delacretaz commented on SLING-3223:
I haven't looked at your code yet but the following:
{code}
HTTP GET http://host:port/system/replication/agent/configuration/foo for
retrieving configuration of agent foo
HTTP POST (/w parameters) to
http://host:port/system/replication/agent/configuration/foo for updating
configuration of agent foo
{code}
Makes me think you're using your own configuration mechanism instead of using
OSGi configs, as Toby also notes. Is there a reason for that? Otherwise, using
OSGi configs should allow you to get rid of some code, and reuse the standard
webconsole and other tools to manage configs.
But all this can be handled once your code is donated, IMO it doesn't prevent
us from committing your code once the paperwork is ok.
Donation of a replication module for Sling
--
Key: SLING-3223
URL: https://issues.apache.org/jira/browse/SLING-3223
Project: Sling
Issue Type: Task
Components: Extensions
Reporter: Tommaso Teofili
Attachments: SLING-3223.patch
Issue to track donation of a replication module for Sling, see thread at
http://markmail.org/thread/ic62k5pc34ppb5ko
--
This message was sent by Atlassian JIRA
(v6.1#6144)
[jira] [Commented] (SLING-3223) Donation of a replication module for Sling
[
https://issues.apache.org/jira/browse/SLING-3223?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13814764#comment-13814764
]
Tommaso Teofili commented on SLING-3223:
bq. HTTP POST (/w parameters) to
http://host:port/system/replication/agent/configuration/foo for updating
configuration of agent foo
typo there, sorry, it's :
{code}
HTTP POST (/w parameters) to
http://host:port/system/replication/agent/foo/configuration for updating
configuration of agent foo
{code}
bq. Makes me think you're using your own configuration mechanism instead of
using OSGi configs, as Toby also notes. Is there a reason for that? Otherwise,
using OSGi configs should allow you to get rid of some code, and reuse the
standard webconsole and other tools to manage configs.
I'm using OSGi configs (configuration factory /w sling:OsgiConfig nodes) for
creating configurations then for retrieving / updating configurations I use a
resource provider which maps the HTTP request to the resource
{code}
/system/replication/agent/foo/configuration
{code}
first to the agent /system/replication/agent/foo then I use ConfigurationAdmin
to get the OSGi config for that agent.
Maybe (probably, at this point :) ) I'm not aware of simpler existing mechanism
for retrieving / updating OSGi configurations so a pointer there would be good.
bq. But all this can be handled once your code is donated, IMO it doesn't
prevent us from committing your code once the paperwork is ok.
good
Donation of a replication module for Sling
--
Key: SLING-3223
URL: https://issues.apache.org/jira/browse/SLING-3223
Project: Sling
Issue Type: Task
Components: Extensions
Reporter: Tommaso Teofili
Attachments: SLING-3223.patch
Issue to track donation of a replication module for Sling, see thread at
http://markmail.org/thread/ic62k5pc34ppb5ko
--
This message was sent by Atlassian JIRA
(v6.1#6144)
[jira] [Commented] (SLING-3223) Donation of a replication module for Sling
[ https://issues.apache.org/jira/browse/SLING-3223?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13815155#comment-13815155 ] Tobias Bocanegra commented on SLING-3223: - btw: how is deactivate implemented? not via an empty content package? an empty package (see VoidReplicationPackage) but then, is there really a difference between _activate_ and _deacticate_ ? they \[_the authentication handlers_] are used for authenticate TransportHandlers which may deliver content to either an HTTP endpoint (most often) or not (e.g. file system, or other) ok. consider using a different name, then. the adapter from SlingRequest to ReplicationPackage is really questionable in IMO. It is used in a very specific case and the adaption is considerably expensive. Or do you foresee that other clients of this API adapt a request to a ReplicationPackage ? I agree that can be much improved, that's just used from the receiving servlet for reading a replication package stream to install the package on the receiving instance. I would get rid of it and put it into an utility class, or into the servlet where you need it. it just pollutes the set of adapters in sling. Donation of a replication module for Sling -- Key: SLING-3223 URL: https://issues.apache.org/jira/browse/SLING-3223 Project: Sling Issue Type: Task Components: Extensions Reporter: Tommaso Teofili Attachments: SLING-3223.patch Issue to track donation of a replication module for Sling, see thread at http://markmail.org/thread/ic62k5pc34ppb5ko -- This message was sent by Atlassian JIRA (v6.1#6144)
[jira] [Commented] (SLING-3223) Donation of a replication module for Sling
[
https://issues.apache.org/jira/browse/SLING-3223?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13814130#comment-13814130
]
Tobias Bocanegra commented on SLING-3223:
-
some quick comments:
- way to little documentation (javadoc, general architecture)
- Don't use HTTP headers to transport additional information. this makes it
harder to get request correctly though proxies and firewalls
- If you use customer HTTP headers, it's best practice to prepend an app
specific prefix, eg: {{X-Sling-Replication-Action}}.
- why do you expose a {{ReplicationConfigurationServlet}} and not rely on
default mechanisms to update config, e.g. update the config via
SlingPostServlet or via felix console? (Is there some functionality missing in
Sling or does every component needs to provide it's own management REST API?)
- {{ReplicationActionType}} names are probably coming from Adobe's replication
- I would reconsider if _Activate_ and _Deactivate_ really are good names. btw:
how is _deactivate_ implemented? not via an empty content package?
-- alternative names for activate: add, put, import, post
-- alternative names for deactivate: delete
- I don't know if {{TriggerPathReplicationRule}} is really a sufficient filter.
Usually it's hard to determined at what point a modified subtree is ready to
replicate. you would probably need to add much more logic to it
- I don't quite understand {{AuthenticationHandler}}. Are those providers that
are used to add authentication to the HTTP requests, or are they consumers that
are used to authenticate replication payloads?
-- If the first, I would rename them to something more specific, like:
{{ReplicationCredentialsProvider}}
-- If the latter, I would really not reinvent the wheel here, and rely on
authentication already present in Sling.
- the adapter from {{SlingRequest}} to {{ReplicationPackage}} is really
questionable in IMO. It is used in a very specific case and the adaption is
considerably expensive. Or do you foresee that other clients of this API adapt
a request to a ReplicationPackage ?
- suggest to add package-info.java files with the bundle export information
over Export-Package elements in the pom.xml
Donation of a replication module for Sling
--
Key: SLING-3223
URL: https://issues.apache.org/jira/browse/SLING-3223
Project: Sling
Issue Type: Task
Components: Extensions
Reporter: Tommaso Teofili
Attachments: SLING-3223.patch
Issue to track donation of a replication module for Sling, see thread at
http://markmail.org/thread/ic62k5pc34ppb5ko
--
This message was sent by Atlassian JIRA
(v6.1#6144)
