Re: HealthCheckExecutor.execute(ServiceReference) ??
I agree, I would definitely not force implementors to provide the method HealthCheck.getMetadata() - I believe it's rarely a good idea to provide meta data programmatically (if we really wanted to have a unique ID, we could add it as service properties like the name or the tags). Additionally, forcing an implementor to define an id for a health check would feel awkward to 99% of the people, as they only provide them but never use it because the hypothetical signature HealthCheckExecutor.execute(hcId) would normally only be used by JMX. -Georg Am 24.12.2013 12:11, schrieb Carsten Ziegeler: It's not just about the metadata - it's about uniquely identifying a HC within the executor - the service object is not a good key, especially as we have no way to discard it from the cache. The service reference or the service id contained within the reference is a good key. And uniquely identifying (for long running hcs) and caching the hc result is for me the number one reason for the executor service. We should not change the HC interface as this is an incompatible change and would break all existing implementations - as we've discussed during defining the first version of the API, there is no real need to have the name, tags etc. in the HC interface. Services executing an HC get the service anyway from the service registry together with these registration properties and it makes the implementation unnecessarily complicated. So we need something to uniquely identify a health check and as all metadata is optional we don't have anything to be used. Now, as I wrote in the issue, I'm fine with having an execute(String name) method which tries to execute a HC with the given name. This would require that a HC which can be executed by this service either has a tag or a name. On the other hand, if we go with ServiceReference we can execute all HCs through this service. And as Georg has pointed out, there are not many clients of this service anyway: its the jmx implementation and the web console. Carsten 2013/12/24 Bertrand Delacretaz bdelacre...@apache.org Hi, About SLING-3278 - IMO execute(ServiceReference) at [1] is an unnecessary leak of implementation details, it makes no sense at the API level. It's HealthChecks that we are executing, not service references. IIUC the root cause is that the HealthCheck API doesn't provide metadata that it should: tags, name, optional JMX MBean name, unique ID maybe, all these things clearly belong to a HealthCheck, so the API should provide access to them, even if they are defined by service properties. I suggest adding a HealthCheckMetadata interface to provide those things (tags, name, optional things in a Map maybe), and add a HealthCheck.getMetadata() method that returns this. That'll break existing HealthCheck implementations, which we indicate by bumping up the package version number, as this quite a new API it's better to fix it now than later. -Bertrand [1] https://svn.apache.org/repos/asf/sling/trunk/bundles/extensions/healthcheck/core/src/main/java/org/apache/sling/hc/api/HealthCheckExecutor.java
Re: HealthCheckExecutor.execute(ServiceReference) ??
Hi Carsten and Georg, I was going to say that I still disagree, but being outnumbered I'd let it go if it's just about a single method that I don't like...and then I reviewed the org.apache.sling.hc.api package once again and it's worse than I thought. You guys both say you don't want metadata in APIs...and the HealthCheckResult interface that you added has these two methods (which make sense BTW): String getHealthCheckName(); ListString getHealthCheckTags(); So a HC is not brave enough to have this in its API, but a Result needs them? This clearly demonstrates that we need a HealthCheckMetadata interface that provides those values. On top of that, the Result throws UnsupportedException on these methods...how ugly is that, why can't it provide these values from the HealthCheck that was used to create it? To me this smells of implementation-driven APIs, we are used to better design in Sling. What do others think? -Bertrand [1] https://svn.apache.org/repos/asf/sling/trunk/bundles/extensions/healthcheck/core/src/main/java/org/apache/sling/hc/api
[jira] [Commented] (SLING-3278) Provide a HealthCheckExecutor service
[ https://issues.apache.org/jira/browse/SLING-3278?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13857487#comment-13857487 ] Bertrand Delacretaz commented on SLING-3278: Also, for my use cases I'll need a way to selectively clear cached results and specify a timeout when executing HCs. We can discuss this separately (started that in [1]), just wanted to mention it here so we don't forget. [1] http://sling.markmail.org/thread/xg4k2pu4ii7xdgbw Provide a HealthCheckExecutor service - Key: SLING-3278 URL: https://issues.apache.org/jira/browse/SLING-3278 Project: Sling Issue Type: New Feature Components: Health Check Reporter: Georg Henzler Assignee: Carsten Ziegeler Attachments: SLING-3278-bertrand.patch, SLING-3278-hc.core-HealthCheckExecutorService-2013-12-19.patch, SLING-3278-hc.core-HealthCheckExecutorService-2013-12-21-withExecutorResult.patch, SLING-3278-hc.webconsole-2013-12-19.patch, SLING-3278-hc.webconsole-2013-12-21.patch, hc-it.patch Goals: * Be able to get an overall (aggregated) result as quickly as possible (ideally 2sec) * Whenever possible, return most current results (e.g. for a memory check) * Provide a declarative way for async checks (async checks should be the exception though) Approach * Run checks in parallel * Make sure long running (or even stuck) checks are timed out * If a health check must run asynchronously (because its execution time cannot be optimized), it should be enough to just specify a service property (e.g. hc.async). See also http://apache-sling.73963.n3.nabble.com/Health-Check-Improvements-td4029330.html#a4029402 http://apache-sling.73963.n3.nabble.com/Health-checks-execution-service-td4028477.html -- This message was sent by Atlassian JIRA (v6.1.5#6160)
Re: [RFC] YAMF - Yet Another Model Factory
Hi Justin, Finally found time to review YAMF. On Thu, Dec 19, 2013 at 6:07 PM, Justin Edelson jus...@justinedelson.com wrote: ...I'm calling this YAMF for now, although ideally we'll just call it Sling Models... +1 for Sling Models, that's consistent with our general naming. My comments, after looking at the wiki page only: 1) Should it be @SlingModel rather than @Model? By analogy with @SlingServlet. 2) For adaptation, would my SLING-2938 adapter methods prototype make sense? 3) Is it a good idea to use @Inject for both services and resource values? Using @ResourceValue for the latter makes it easier to explain both IMO. 4) Also, would @Path make sense instead of @Named? Does the implementation then support @Path(child-node/some-property) ? 5) @Projection is not obvious to me from its name, how about @ChildPath ? Naming nitpicks mostly, YAMF looks good to me, thanks! -Bertrand
Re: [RT] New Feature Flags API / Impl
Hi Carsten, On Thu, Dec 19, 2013 at 6:02 AM, Carsten Ziegeler cziege...@apache.org wrote: ...Everything can be found at: http://svn.apache.org/repos/asf/sling/whiteboard/feature-flags/ ... Today is Big Reviews Day, finally found time to look at that, thanks for it! It looks good to me for our current needs, here are my comments: 1) Why not put the API in the org.apache.sling.extensions.featureflags.api package? It's at org.apache.sling.extensions.featureflags currently. 2) How do you envision handling feature naming collisions between FeatureProviders? We might namespace feature names by requiring all values returned by the getFeatureNames() method of a given FeatureProvider to start with a common prefix followed by a dot, and the prefix must be unique system-wide. 3) Considering Roy's remark that we'll get about 1024 features in a Sling instance, and user's creativity which will probably boost that to 10'000, shall we use IteratorString for lists of feature names? 4) I'm not sure about FeatureProvider.hideResource(featureName, Resource) - as seen from the ResourceResolver, IMO you just care whether a given FeatureProvider wants to hide a Resource or not, you don't really care (*) which feature name causes the Resource to be hidden. So maybe just hideResource(Resource, ProviderContext) ? The same might apply to FeatureProvider.getResourceTypeMapping which would then just take a ProviderContext - but maybe I'm missing something. -Bertrand (*) Except for troubleshooting, but that's covered by RequestProgressTracker which is accessible via ProviderContext.getRequest()
[jira] [Comment Edited] (SLING-3278) Provide a HealthCheckExecutor service
[ https://issues.apache.org/jira/browse/SLING-3278?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13857487#comment-13857487 ] Bertrand Delacretaz edited comment on SLING-3278 at 12/27/13 2:18 PM: -- Also, for my use cases I'll need a way to selectively clear cached results and specify a timeout when executing HCs and get an aggregated result. We can discuss this separately (started that in [1]), just wanted to mention it here so we don't forget. [1] http://sling.markmail.org/thread/xg4k2pu4ii7xdgbw was (Author: bdelacretaz): Also, for my use cases I'll need a way to selectively clear cached results and specify a timeout when executing HCs. We can discuss this separately (started that in [1]), just wanted to mention it here so we don't forget. [1] http://sling.markmail.org/thread/xg4k2pu4ii7xdgbw Provide a HealthCheckExecutor service - Key: SLING-3278 URL: https://issues.apache.org/jira/browse/SLING-3278 Project: Sling Issue Type: New Feature Components: Health Check Reporter: Georg Henzler Assignee: Carsten Ziegeler Attachments: SLING-3278-bertrand.patch, SLING-3278-hc.core-HealthCheckExecutorService-2013-12-19.patch, SLING-3278-hc.core-HealthCheckExecutorService-2013-12-21-withExecutorResult.patch, SLING-3278-hc.webconsole-2013-12-19.patch, SLING-3278-hc.webconsole-2013-12-21.patch, hc-it.patch Goals: * Be able to get an overall (aggregated) result as quickly as possible (ideally 2sec) * Whenever possible, return most current results (e.g. for a memory check) * Provide a declarative way for async checks (async checks should be the exception though) Approach * Run checks in parallel * Make sure long running (or even stuck) checks are timed out * If a health check must run asynchronously (because its execution time cannot be optimized), it should be enough to just specify a service property (e.g. hc.async). See also http://apache-sling.73963.n3.nabble.com/Health-Check-Improvements-td4029330.html#a4029402 http://apache-sling.73963.n3.nabble.com/Health-checks-execution-service-td4028477.html -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Updated] (SLING-3027) improve Launchpad Karaf features
[ https://issues.apache.org/jira/browse/SLING-3027?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Oliver Lietz updated SLING-3027: Description: - break down sling-karaf feature into more fine-grained features for better reusability - add descriptions and versions - add more features - update integration tests - update Pax Exam to 3.4.0 - update Karaf to 3.0.0 - update Maven Failsafe Plugin to 2.16 - add workaround for KARAF-1972 - update README - cleanup was: - break down sling-karaf feature into more fine-grained features for better reusability - add descriptions and versions - add more features - update integration tests - update Pax Exam to 3.3.0 - update Maven Failsafe Plugin to 2.16 - add workaround for KARAF-1972 - update README - cleanup improve Launchpad Karaf features Key: SLING-3027 URL: https://issues.apache.org/jira/browse/SLING-3027 Project: Sling Issue Type: Improvement Components: Launchpad Reporter: Oliver Lietz Assignee: Oliver Lietz Priority: Minor - break down sling-karaf feature into more fine-grained features for better reusability - add descriptions and versions - add more features - update integration tests - update Pax Exam to 3.4.0 - update Karaf to 3.0.0 - update Maven Failsafe Plugin to 2.16 - add workaround for KARAF-1972 - update README - cleanup -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Commented] (SLING-3234) Sling buildbot continuous integration
[ https://issues.apache.org/jira/browse/SLING-3234?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13857557#comment-13857557 ] Bertrand Delacretaz commented on SLING-3234: http://ci.apache.org/builders/sling-trunk-oak/builds/64 has failed with something that looks like an infrastructure problem - for now, just making a note of it in case that happens again. {code} [ERROR] Failed to execute goal org.apache.maven.plugins:maven-install-plugin:2.3.1:install (default-install) on project org.apache.sling.settings: Failed to install metadata org.apache.sling:org.apache.sling.settings:1.3.1-SNAPSHOT/maven-metadata.xml: Could not read metadata /home/buildslave21/.m2/repository/org/apache/sling/org.apache.sling.settings/1.3.1-SNAPSHOT/maven-metadata-local.xml: input contained no data - [Help 1] {code} Sling buildbot continuous integration - Key: SLING-3234 URL: https://issues.apache.org/jira/browse/SLING-3234 Project: Sling Issue Type: Task Components: Testing Reporter: Bertrand Delacretaz Assignee: Bertrand Delacretaz Buildbot jobs have been setup for Sling in INFRA-6962; * http://ci.apache.org/builders/sling-trunk (triggered by commits) * http://ci.apache.org/builders/sling-trunk-oak (runs after sling-trunk) We'll use this issue to keep track of changes related to this buildbot setup, as we did with SLING-920 for our Jenkins builds. I suggest marking issues that cause intermittent buildbot failures as blockers for this issue, so that we can quickly check if failed builds are due to known issues. Builds can be started manually via IRC if needed, via #asftest. Talk to the sling-bot there, start with help force, status or commands. [1] https://svn.apache.org/repos/infra/infrastructure/buildbot/aegis/buildmaster/master1/projects/sling.conf -- This message was sent by Atlassian JIRA (v6.1.5#6160)
Build failed in Jenkins: sling-contrib-1.6 ยป Apache Sling JCR Resource Security #1111
See https://builds.apache.org/job/sling-contrib-1.6/org.apache.sling$org.apache.sling.jcr.resourcesecurity// -- [INFO] [INFO] [INFO] Building Apache Sling JCR Resource Security 0.0.1-SNAPSHOT [INFO] Downloading: http://repository.apache.org/snapshots/org/apache/sling/org.apache.sling.resourceaccesssecurity/0.0.1-SNAPSHOT/maven-metadata.xml Downloading: http://repository.apache.org/snapshots/org/apache/sling/org.apache.sling.resourceaccesssecurity/0.0.1-SNAPSHOT/org.apache.sling.resourceaccesssecurity-0.0.1-SNAPSHOT.pom [WARNING] The POM for org.apache.sling:org.apache.sling.resourceaccesssecurity:jar:0.0.1-SNAPSHOT is missing, no dependency information available Downloading: http://repository.apache.org/snapshots/org/apache/sling/org.apache.sling.resourceaccesssecurity/0.0.1-SNAPSHOT/org.apache.sling.resourceaccesssecurity-0.0.1-SNAPSHOT.jar
Build failed in Jenkins: sling-contrib-1.6 #1111
See https://builds.apache.org/job/sling-contrib-1.6//changes Changes: [olli] SLING-3027 update Karaf to 3.0.0 and Pax Exam to 3.4.0 -- [...truncated 4571 lines...] [echo] On most platforms, you'll get OutOfMemoryErrors when building unless you set [echo] on 32bit platforms: MAVEN_OPTS=-Xmx256M -XX:MaxPermSize=256M, see SLING-443 [echo] on 64bit platforms: MAVEN_OPTS=-Xmx512M -XX:MaxPermSize=512M, see SLING-1782 [echo] ** [INFO] Executed tasks [INFO] [INFO] --- maven-compiler-plugin:2.5.1:compile (default-compile) @ org.apache.sling.jcr.compiler --- [INFO] Compiling 2 source files to https://builds.apache.org/job/sling-contrib-1.6/ws/contrib-1.6/jcr/compiler/target/classes [INFO] [INFO] --- maven-scr-plugin:1.14.0:scr (generate-scr-scrdescriptor) @ org.apache.sling.jcr.compiler --- [INFO] Writing 1 Service Component Descriptors to https://builds.apache.org/job/sling-contrib-1.6/ws/contrib-1.6/jcr/compiler/target/scr-plugin-generated/OSGI-INF/org.apache.sling.jcr.compiler.impl.JcrJavaCompilerImpl.xml [INFO] [INFO] --- maven-resources-plugin:2.6:testResources (default-testResources) @ org.apache.sling.jcr.compiler --- [INFO] Using 'UTF-8' encoding to copy filtered resources. [INFO] skip non existing resourceDirectory https://builds.apache.org/job/sling-contrib-1.6/ws/contrib-1.6/jcr/compiler/src/test/resources [INFO] Copying 3 resources [INFO] [INFO] --- maven-compiler-plugin:2.5.1:testCompile (default-testCompile) @ org.apache.sling.jcr.compiler --- [INFO] No sources to compile [INFO] [INFO] --- maven-surefire-plugin:2.12.4:test (default-test) @ org.apache.sling.jcr.compiler --- [JENKINS] Recording test results[INFO] [INFO] --- animal-sniffer-maven-plugin:1.9:check (default) @ org.apache.sling.jcr.compiler --- [INFO] Checking unresolved references to org.codehaus.mojo.signature:java15:1.0 [INFO] [INFO] --- maven-bundle-plugin:2.4.0:bundle (default-bundle) @ org.apache.sling.jcr.compiler --- [INFO] [INFO] --- maven-enforcer-plugin:1.0.1:enforce (enforce-java) @ org.apache.sling.jcr.compiler --- [INFO] [INFO] --- maven-antrun-plugin:1.7:run (set-bundle-required-execution-environment) @ org.apache.sling.jcr.compiler --- [INFO] Executing tasks main: [INFO] Executed tasks [WARNING] Failed to getClass for org.apache.maven.plugin.source.SourceJarMojo [INFO] [INFO] --- maven-source-plugin:2.2.1:jar (attach-sources) @ org.apache.sling.jcr.compiler --- [INFO] Building jar: https://builds.apache.org/job/sling-contrib-1.6/ws/contrib-1.6/jcr/compiler/target/org.apache.sling.jcr.compiler-2.1.1-SNAPSHOT-sources.jar [INFO] [INFO] --- maven-site-plugin:3.2:attach-descriptor (attach-descriptor) @ org.apache.sling.jcr.compiler --- [INFO] [INFO] --- ianal-maven-plugin:1.0-alpha-1:verify-legal-files (default) @ org.apache.sling.jcr.compiler --- [INFO] Checking legal files in: org.apache.sling.jcr.compiler-2.1.1-SNAPSHOT.jar [INFO] Checking legal files in: org.apache.sling.jcr.compiler-2.1.1-SNAPSHOT-sources.jar [INFO] [INFO] --- maven-install-plugin:2.3.1:install (default-install) @ org.apache.sling.jcr.compiler --- [INFO] Installing https://builds.apache.org/job/sling-contrib-1.6/ws/contrib-1.6/jcr/compiler/target/org.apache.sling.jcr.compiler-2.1.1-SNAPSHOT.jar to /home/jenkins/jenkins-slave/maven-repositories/0/org/apache/sling/org.apache.sling.jcr.compiler/2.1.1-SNAPSHOT/org.apache.sling.jcr.compiler-2.1.1-SNAPSHOT.jar [INFO] Installing https://builds.apache.org/job/sling-contrib-1.6/ws/contrib-1.6/jcr/compiler/pom.xml to /home/jenkins/jenkins-slave/maven-repositories/0/org/apache/sling/org.apache.sling.jcr.compiler/2.1.1-SNAPSHOT/org.apache.sling.jcr.compiler-2.1.1-SNAPSHOT.pom [INFO] Installing https://builds.apache.org/job/sling-contrib-1.6/ws/contrib-1.6/jcr/compiler/target/org.apache.sling.jcr.compiler-2.1.1-SNAPSHOT-sources.jar to /home/jenkins/jenkins-slave/maven-repositories/0/org/apache/sling/org.apache.sling.jcr.compiler/2.1.1-SNAPSHOT/org.apache.sling.jcr.compiler-2.1.1-SNAPSHOT-sources.jar [INFO] [INFO] --- maven-bundle-plugin:2.4.0:install (default-install) @ org.apache.sling.jcr.compiler --- [INFO] Local OBR update disabled (enable with -DobrRepository) [INFO] [INFO] [INFO] Building Apache Sling JCR Prefs 1.0.1-SNAPSHOT [INFO] [INFO] [INFO] --- maven-clean-plugin:2.5:clean (default-clean) @ org.apache.sling.jcr.prefs --- [INFO] [INFO] --- maven-enforcer-plugin:1.0.1:enforce (enforce-java) @ org.apache.sling.jcr.prefs --- [INFO] [INFO] --- maven-antrun-plugin:1.7:run (set-bundle-required-execution-environment) @ org.apache.sling.jcr.prefs --- [INFO] Executing tasks main: [INFO] Executed tasks [INFO]
Re: [RFC] YAMF - Yet Another Model Factory
Hi Bertrand, On Fri, Dec 27, 2013 at 5:48 AM, Bertrand Delacretaz bdelacre...@apache.org wrote: Hi Justin, Finally found time to review YAMF. On Thu, Dec 19, 2013 at 6:07 PM, Justin Edelson jus...@justinedelson.com wrote: ...I'm calling this YAMF for now, although ideally we'll just call it Sling Models... +1 for Sling Models, that's consistent with our general naming. My comments, after looking at the wiki page only: 1) Should it be @SlingModel rather than @Model? By analogy with @SlingServlet. I thought about this, but honestly the Sling part here is duplicative. sling is already part of the package name, so why include it in the annotation name as well? 2) For adaptation, would my SLING-2938 adapter methods prototype make sense? Possibly, although as I wrote on the wiki page, one of the design goals was to not require changes to existing Sling bundles which AFAICT SLING-2938 does. 3) Is it a good idea to use @Inject for both services and resource values? Using @ResourceValue for the latter makes it easier to explain both IMO. There are already 5 injectors available and I would guess that we need at least one or two others in the core. And other injectors can be added by downstream projects. I don't think it makes sense to have injector-specific annotations unless absolutely necessary (@Filter being the one obvious example of where an annotation is injector-specific). 4) Also, would @Path make sense instead of @Named? Does the implementation then support @Path(child-node/some-property) ? See above. The idea (beyond the goal of using standard annotations where possible) is that @Named works across injector types. @Path doesn't make sense for a binding value or request attribute. 5) @Projection is not obvious to me from its name, how about @ChildPath ? Yeah, I'm not thrilled with @Projection either, but for the same reasons as above @ChildPath won't work consistently. How about @From as in @Model(adaptables = Resource.class) public class Model { @Inject @From(parent) @Named(jcr:title) private String parentTitle; } Which means set the parentTitle field to the jcr:title property of the parent resource (i.e. resource.getParent()). Naming nitpicks mostly, YAMF looks good to me, thanks! I need all the naming help I can get :) Thanks, Justin -Bertrand