[Discuss] - JCR ContentLoader Skip Runmode

[Daniel Klco <dk...@apache.org>]

Hey Sling Team,

As Robert mentioned previously[1] using the JCR Content Loader with a
Composite NodeStore is challenging as it keeps track of the status of the
bundles underneath the /var directory.

One of the practical challenges I've also seen with a Composite Repository
is that you actually want the bundle content to re-install when the
Composite Repository loads. For example, if you want a single bundle to
populate the entire repository you may want it to load content under /apps
during seeding and /conf during runtime.

My proposal would be to add a new PathEntry directive, "skipRunmode" which
would skip the installing of a content entry when the Sling instance is
running with the specified runmode. If not specified the content would
still be loaded so this would be backward compatible.

I've created a ticket[2] and have opened a Pull Request with my recommended
changes[3].

Does anyone see any issues with this approach? Does anyone see value in
supporting this at the bundle level vs path level? Any other ideas?

Regards,
Dan

[1] - https://markmail.org/message/xhirmjeyi6mlv635
[2] - https://issues.apache.org/jira/browse/SLING-9841
[3] -
https://github.com/apache/sling-org-apache-sling-jcr-contentloader/pull/3

Re: [Discuss] - JCR ContentLoader Skip Runmode

[Robert Munteanu <ro...@apache.org>]

On Tue, 2020-10-27 at 13:39 -0400, Daniel Klco wrote:
> Robert / Team,
> 
> Any thoughts / concerns? I would like to get this fix in place.

Hi,

I don't have any major concerns with this approach for the content
loader, so if you want to go down this route please do. 

Thinking out load, I wonder whether it makes sense to go down the route
of the content-package to feature model converter [1] for the content
loader, where we split the bundle in two

- one with code and content for /libs and /apps
- one with content for the rest of the repositor

At any rate, that's a longer-term approach, but I just wanted to
mention it in case someone had interest in such a thing.

Thanks,
Robert

[1]: 
https://github.com/apache/sling-org-apache-sling-feature-cpconverter

> 
> On Thu, Oct 22, 2020 at 2:34 PM Daniel Klco <dk...@apache.org> wrote:
> 
> > Robert,
> > 
> > I agree it could be misconfigured, however the intent is to only
> > configure
> > it when needed and for the code to work the same as the current
> > functionality if not configured.
> > 
> > The challenge I see with requiring the implementer to support
> > separating
> > their content into two bundles is that it pushes the complexity of
> > the
> > internal functionality of the repository onto the implementing team
> > rather
> > than handling it inside the application. We know that /apps and
> > /libs
> > shouldn't be written to at runtime, so why require the user to
> > consider
> > this? Especially when the side-effect is that the entire bundle
> > content
> > fails to install.
> > 
> > I'm very much willing to look into other options, but I'm not a
> > huge fan
> > of having users refactor their codebase to support internal
> > implementation
> > details, especially when it's only required for certain setups.
> > 
> > On Thu, Oct 22, 2020 at 10:12 AM Robert Munteanu <
> > rombert@apache.org>
> > wrote:
> > 
> > > On Wed, 2020-10-21 at 08:25 -0400, Daniel Klco wrote:
> > > > Seeding Setting:
> > > >  - Includes Path: [ "^/apps/.*", "^/libs/.*", "^/oak:index/.*"
> > > > ]
> > > > 
> > > > Runtime Setting:
> > > >  - Includes Path: [ "^/.*" ]
> > > >  - Excludes Path: [ "^/apps/.*", "^/libs/.*", "^/oak:index/.*"
> > > > ]
> > > > 
> > > > The Bundle Content Loader would then filter out the path roots
> > > > based
> > > > on the
> > > > include / exclude rules. I would only expect this to happen at
> > > > the
> > > > path
> > > > root, not for the individual nodes being loaded. The
> > > > configuration
> > > > would
> > > > not be required and in that case the Bundle Content Loader
> > > > would load
> > > > all
> > > > content.
> > > 
> > > I think this will work. I am wondering though whether we are not
> > > opening the door for surprising behaviours and misconfigurations.
> > > I
> > > think the only scenario where this is useful is:
> > > 
> > > - bundle A is used in a composite node store setup
> > > - bundle A contains resources that belong to both the default
> > > mounnt
> > > and the non-default one ( /libs, /apps )
> > > - bundle A installs content using the content loader
> > > 
> > > I think that a better solution would be to avoid this problem
> > > altogether by separating 'code' bundles from 'content' bundles,
> > > and
> > > only installing the 'code' bundle when seeding.
> > > 
> > > Thanks,
> > > Robert
> > > 
> > > 

Re: [Discuss] - JCR ContentLoader Skip Runmode

[Daniel Klco <dk...@apache.org>]

Robert / Team,

Any thoughts / concerns? I would like to get this fix in place.

On Thu, Oct 22, 2020 at 2:34 PM Daniel Klco <dk...@apache.org> wrote:

> Robert,
>
> I agree it could be misconfigured, however the intent is to only configure
> it when needed and for the code to work the same as the current
> functionality if not configured.
>
> The challenge I see with requiring the implementer to support separating
> their content into two bundles is that it pushes the complexity of the
> internal functionality of the repository onto the implementing team rather
> than handling it inside the application. We know that /apps and /libs
> shouldn't be written to at runtime, so why require the user to consider
> this? Especially when the side-effect is that the entire bundle content
> fails to install.
>
> I'm very much willing to look into other options, but I'm not a huge fan
> of having users refactor their codebase to support internal implementation
> details, especially when it's only required for certain setups.
>
> On Thu, Oct 22, 2020 at 10:12 AM Robert Munteanu <ro...@apache.org>
> wrote:
>
>> On Wed, 2020-10-21 at 08:25 -0400, Daniel Klco wrote:
>> > Seeding Setting:
>> >  - Includes Path: [ "^/apps/.*", "^/libs/.*", "^/oak:index/.*" ]
>> >
>> > Runtime Setting:
>> >  - Includes Path: [ "^/.*" ]
>> >  - Excludes Path: [ "^/apps/.*", "^/libs/.*", "^/oak:index/.*" ]
>> >
>> > The Bundle Content Loader would then filter out the path roots based
>> > on the
>> > include / exclude rules. I would only expect this to happen at the
>> > path
>> > root, not for the individual nodes being loaded. The configuration
>> > would
>> > not be required and in that case the Bundle Content Loader would load
>> > all
>> > content.
>>
>> I think this will work. I am wondering though whether we are not
>> opening the door for surprising behaviours and misconfigurations. I
>> think the only scenario where this is useful is:
>>
>> - bundle A is used in a composite node store setup
>> - bundle A contains resources that belong to both the default mounnt
>> and the non-default one ( /libs, /apps )
>> - bundle A installs content using the content loader
>>
>> I think that a better solution would be to avoid this problem
>> altogether by separating 'code' bundles from 'content' bundles, and
>> only installing the 'code' bundle when seeding.
>>
>> Thanks,
>> Robert
>>
>>

Re: [Discuss] - JCR ContentLoader Skip Runmode

[Daniel Klco <dk...@apache.org>]

Robert,

I agree it could be misconfigured, however the intent is to only configure
it when needed and for the code to work the same as the current
functionality if not configured.

The challenge I see with requiring the implementer to support separating
their content into two bundles is that it pushes the complexity of the
internal functionality of the repository onto the implementing team rather
than handling it inside the application. We know that /apps and /libs
shouldn't be written to at runtime, so why require the user to consider
this? Especially when the side-effect is that the entire bundle content
fails to install.

I'm very much willing to look into other options, but I'm not a huge fan of
having users refactor their codebase to support internal implementation
details, especially when it's only required for certain setups.

On Thu, Oct 22, 2020 at 10:12 AM Robert Munteanu <ro...@apache.org> wrote:

> On Wed, 2020-10-21 at 08:25 -0400, Daniel Klco wrote:
> > Seeding Setting:
> >  - Includes Path: [ "^/apps/.*", "^/libs/.*", "^/oak:index/.*" ]
> >
> > Runtime Setting:
> >  - Includes Path: [ "^/.*" ]
> >  - Excludes Path: [ "^/apps/.*", "^/libs/.*", "^/oak:index/.*" ]
> >
> > The Bundle Content Loader would then filter out the path roots based
> > on the
> > include / exclude rules. I would only expect this to happen at the
> > path
> > root, not for the individual nodes being loaded. The configuration
> > would
> > not be required and in that case the Bundle Content Loader would load
> > all
> > content.
>
> I think this will work. I am wondering though whether we are not
> opening the door for surprising behaviours and misconfigurations. I
> think the only scenario where this is useful is:
>
> - bundle A is used in a composite node store setup
> - bundle A contains resources that belong to both the default mounnt
> and the non-default one ( /libs, /apps )
> - bundle A installs content using the content loader
>
> I think that a better solution would be to avoid this problem
> altogether by separating 'code' bundles from 'content' bundles, and
> only installing the 'code' bundle when seeding.
>
> Thanks,
> Robert
>
>

Re: [Discuss] - JCR ContentLoader Skip Runmode

[Robert Munteanu <ro...@apache.org>]

On Wed, 2020-10-21 at 08:25 -0400, Daniel Klco wrote:
> Seeding Setting:
>  - Includes Path: [ "^/apps/.*", "^/libs/.*", "^/oak:index/.*" ]
> 
> Runtime Setting:
>  - Includes Path: [ "^/.*" ]
>  - Excludes Path: [ "^/apps/.*", "^/libs/.*", "^/oak:index/.*" ]
> 
> The Bundle Content Loader would then filter out the path roots based
> on the
> include / exclude rules. I would only expect this to happen at the
> path
> root, not for the individual nodes being loaded. The configuration
> would
> not be required and in that case the Bundle Content Loader would load
> all
> content.

I think this will work. I am wondering though whether we are not
opening the door for surprising behaviours and misconfigurations. I
think the only scenario where this is useful is:

- bundle A is used in a composite node store setup
- bundle A contains resources that belong to both the default mounnt
and the non-default one ( /libs, /apps )
- bundle A installs content using the content loader

I think that a better solution would be to avoid this problem
altogether by separating 'code' bundles from 'content' bundles, and
only installing the 'code' bundle when seeding.

Thanks,
Robert

Re: [Discuss] - JCR ContentLoader Skip Runmode

[Daniel Klco <dk...@apache.org>]

That's why I wanted to discuss the approach. I was trying to think of ways
to link the contents of the Bundle Content to the instance state, Runmodes
seem like a logical solution.

On further reflection this morning, I think it makes more sense to handle
this with include / exclude settings in the JCR Content Loader. The idea
being you could have an OSGi Config with:

Seeding Setting:
 - Includes Path: [ "^/apps/.*", "^/libs/.*", "^/oak:index/.*" ]

Runtime Setting:
 - Includes Path: [ "^/.*" ]
 - Excludes Path: [ "^/apps/.*", "^/libs/.*", "^/oak:index/.*" ]

The Bundle Content Loader would then filter out the path roots based on the
include / exclude rules. I would only expect this to happen at the path
root, not for the individual nodes being loaded. The configuration would
not be required and in that case the Bundle Content Loader would load all
content.

I would think this solution would work and adds the added benefit of being
very backwards compatible and not requiring changes to the bundle
configurations.

WDYT?

On Wed, Oct 21, 2020 at 5:31 AM Robert Munteanu <ro...@apache.org> wrote:

> Hi Daniel,
>
> On Mon, 2020-10-19 at 14:49 -0400, Daniel Klco wrote:
> > Hey Sling Team,
> >
> > As Robert mentioned previously[1] using the JCR Content Loader with a
> > Composite NodeStore is challenging as it keeps track of the status of
> > the
> > bundles underneath the /var directory.
> >
> > One of the practical challenges I've also seen with a Composite
> > Repository
> > is that you actually want the bundle content to re-install when the
> > Composite Repository loads. For example, if you want a single bundle
> > to
> > populate the entire repository you may want it to load content under
> > /apps
> > during seeding and /conf during runtime.
> >
> > My proposal would be to add a new PathEntry directive, "skipRunmode"
> > which
> > would skip the installing of a content entry when the Sling instance
> > is
> > running with the specified runmode. If not specified the content
> > would
> > still be loaded so this would be backward compatible.
>
> Without looking at the code - why have you tried linking this to
> runmodes? We are trying to move away from that and with the feature
> model you can have different configurations per feature ( or aggregate
> ).
>
> Thanks,
> Robert
>
>

Re: [Discuss] - JCR ContentLoader Skip Runmode

[Robert Munteanu <ro...@apache.org>]

Hi Daniel,

On Mon, 2020-10-19 at 14:49 -0400, Daniel Klco wrote:
> Hey Sling Team,
> 
> As Robert mentioned previously[1] using the JCR Content Loader with a
> Composite NodeStore is challenging as it keeps track of the status of
> the
> bundles underneath the /var directory.
> 
> One of the practical challenges I've also seen with a Composite
> Repository
> is that you actually want the bundle content to re-install when the
> Composite Repository loads. For example, if you want a single bundle
> to
> populate the entire repository you may want it to load content under
> /apps
> during seeding and /conf during runtime.
> 
> My proposal would be to add a new PathEntry directive, "skipRunmode"
> which
> would skip the installing of a content entry when the Sling instance
> is
> running with the specified runmode. If not specified the content
> would
> still be loaded so this would be backward compatible.

Without looking at the code - why have you tried linking this to
runmodes? We are trying to move away from that and with the feature
model you can have different configurations per feature ( or aggregate
).

Thanks,
Robert