Re: HealthCheckExecutor.execute(ServiceReference) ??

2013-12-27 Thread Georg Henzler
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) ??

2013-12-27 Thread Bertrand Delacretaz
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

2013-12-27 Thread Bertrand Delacretaz (JIRA)

[ 
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

2013-12-27 Thread Bertrand Delacretaz
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

2013-12-27 Thread Bertrand Delacretaz
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

2013-12-27 Thread Bertrand Delacretaz (JIRA)

[ 
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

2013-12-27 Thread Oliver Lietz (JIRA)

 [ 
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

2013-12-27 Thread Bertrand Delacretaz (JIRA)

[ 
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

2013-12-27 Thread Apache Jenkins Server
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

2013-12-27 Thread Apache Jenkins Server
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

2013-12-27 Thread Justin Edelson
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