On Thu, Apr 1, 2010 at 16:58, Lin Sun <[email protected]> wrote:
> Hi Guillaume, thanks for the reply. I have some comments inline.
>
> Lin
>
> On Wed, Mar 31, 2010 at 7:42 PM, Guillaume Nodet <[email protected]> wrote:
> >> The first question that comes to my mind is about the use of the
> >> scope/identifier.
> >> I don't have any problem adding such an identifier to a subsystem (or
> >> replace the
> >> long identifier by a string), however, I'm concerned about using the
> >> symbolic name
> >> + version as the identifier. The problem is that when updating the
> >> subsystem with
> >> a more recent version, the id would either change (which would be
> >> problematic) or
> >> become inconsistent (which would be bad too).
>
> CompositeBundle.update() or update(inputstream) is not supported.
> So seems to me we would have to uninstall the subsystem and reinstall
> it for update. Do you see any other way to do this?
> I was thinking that the subsystem symbolic name would be same as the
> composite bundle's symbolic name and subsystem's version would be the
> same as composite bundle's version, thus within a single physical
> framework, symbolic name + version should be unique.
>
The CompositeBundle does not support update because there's no archive
as for the bundle. However, it does support an update(Map<String,String>)
which updates the manifest (hence the import/export policies for services
and
packages). I was thinking about leveraging that in addition to updating
bundles,
installing new bundles, removing old bundles, etc...
So it would basically behave the same as a bundle, which means that when you
update the subsystem, the symbolic name or version could change, though
it has to be unique within the framework.
This would mean that the symbolic name + version could change over time :-(
>
> >> In addition a user could
> >> choose
> >> to have the same subsystem (identified by its symbolic name) running in
> two
> >> different versions. The question becomes what would happen if the user
> try
> >> to
> >> update the first one using the more up to date subsystem. For bundles,
> it
> >> would
> >> fail afaik.
>
> I think this case would fail for composite too as composite is a
> regular bundle and the framework doesn't allow bundles who have the
> same bundle symbolic name and version co-exist.
>
Yeah, i think so too.
>
> >> In addition, the id would have been mapped to the underlying
> >> composite
> >> id, leading to a simple way to access the composite if needed.
>
> I agree, if we use id, we would use the composite id in the framework,
> which should be unique. You are right that having the id gives us a
> simple way to access the composite, but how would user get the id in
> the first place? It seems user would need to iterate through all
> bundles in the framework and find the bundle they need by checking the
> bundle symbolic name and version (which they can already get hold of
> the composite).
>
Not sure what's your use case is. How does the user start a bundle ?
He has to know the id anyway, by iterating through bundles and finding
the one he is interested in (might be using the symbolic name, or using
the name and/or description).
>
> >> So i'd like to understand how you see this scope being used. If you
> want a
> >> generated id,
> >> then what about using a UUID ? If what you want is something that can
> >> easily be undestand
> >> and linked to the symbolicname / version, then is that really an unique
> >> identifier ? because it
> >> may change over time. Actually, I think it would be ok to update the
> >> subsystem and have
> >> the updated version have a different symbolic name.
>
> The reason I like the scope is that it gives user an easy way to
> access the subsystem when knowing the scope value. When a user is
> requesting an update of the subsystem by providing the new url, we
> need to be able to find the subsystem easily in SubsystemAdmin. We
> can easily grab the scope out of the url by reading the application.mf
> then get the corresponding subsystem. Assuming update of composite
> bundle is not supported, we would uninstall the subsystem, and then
> install a new one, refresh our map.
>
Yeah, I understand. But I think updating a subsystem would be a more
important
feature if possible, and I think it is.
>
> >>>
> >>> 3. Subsystem.install(location). I think this should be install(String
> >>> location, BundleContext bc) instead, because we would need the bundle
> >>> context to get the CompositeAdmin service from OSGi service registry,
> >>> assuming we would use that to install the composite. We would also
> >>> need bundle context to install constituents.
> >>>
> >>> 4. similar as No. 3, Subsyste.install(location, content) should be
> >>> install(String location, InputStream content, BundleContext bc)
> >>>
> >>>
> >> Ok, so I think those two points are related to nested subsystems. My
> idea
> >> was
> >> to follow what composites do, not necessarily because we would use
> those,
> >> but
> >> because I think users will rarely have to deal with nested subsystems
> >> directly.
> >> Let me explain: subsystems are defined by the user and deployed. I
> don't
> >> think
> >> we should encourage users to manually install new bundles into the
> created
> >> subsystem,
> >> because i would assume those would be uninstalled as soon as the
> subsystem
> >> is
> >> updated (or do we want to actually track the changes done by the user so
> >> that we don't
> >> rollback manual changes?). My idea was that if the user want to add /
> >> remove bundles
> >> from the composite, he needs to update the subsystem. I don't think we
> >> should forbid
> >> such changes, just that we should not track and manage those changes.
> >> Nested subsystems are imho the same problem. Nested subsystems would be
> >> installed
> >> because a subsystem references another subsystem in its key content I
> >> think. So you
> >> would not really want the user to manually install a subsystem inside an
> >> existing subsystem.
> >>
> >> Now, if what you were thinking was about installing subsystems from
> inside
> >> a composite,
> >> the problem is a bit different. My idea was that the subsystem would be
> >> installed from inside
> >> the composite where the SubsystemAdmin service you use comes from. So
> if
> >> you have a composite
> >> and you want to install a subsystem into it, you retrieve the
> >> SubsystemAdmin from this composite
> >> and simply call one of the install method. This means that either:
> >> * the subsystem implementation has been installed directly in this
> >> composite
> >> * or the subsystem impl has been installed in a parent composite and
> the
> >> composite service policy has been
> >> written to allow the service goes through
> >> From an implementation pov, I think it's quite easy to achieve by using
> a
> >> ServiceFactory and using the
> >> bundle context from the requesting bundle.
>
> Initially I was not thinking the complicated nested framework case.
> :-) I was just thinking the simple case. Somehow we have to make
> the bundle context available to SubsystemAdmin, so that it can perform
> the work (getting CompositeAdmin out of the service registry, install
> composite, install constituents...). On further thinking, I think
> this can be solved by using FrameworkUtil.getBundle to get the bundle
> then get the bundle context or store the bundle context during
> startup... :)
>
> I agree with you with the case of nested systems, we don't track
> user's install/updates unless it is going through the SubsystemAdmin.
>
> For the 2nd case, I can see we would need to pass in the bundle
> context for install, so that the SubsystemAdmin can get hold of the
> bundle context of the requesting bundle and use that bundle context to
> perform the install (instead of its own bundle context). This is
> needed, in the case we decide to flow the SubsystemAdmin from the
> framework to the nested framework (instead of installing
> SubsystemAdmin service in the nested framework), because the
> SubsystemAdmin would only know the bundle context of the service with
> the main framework.
>
> Would you agree that it makes sense to keep the original 2 install
> methods and also add 2 new install methods to allow bundle context to
> be passed in as a param, in case the bundle context is different than
> the SubsystemAdmin bundle context?
>
No, i really don't think it's needed. Even if we use a single instance of
the SubsystemAdmin
which is passed into the composite, detecting which composite we're in can
be done
by using a ServiceFactory instead. That way, when a bundle requests the
SubsystemAdmin,
you know its bundle, hence it's composite (if any).
So unless we actually want to show and manage the tree of subsystems from
the top level
(which i don't think is a good idea), I don't think we need those methods.
I may be wrong, but i don't think it's technically a problem.
>
>
> >>
> >>> 5. Subsystem.getState(). I wonder if we should reinvent here. Is
> >>> there any reason why we don't make the return type of int, and just
> >>> delegate this to the compositeBundle.getState(). Also, what if the
> >>> composite bundle and constituents have different states. Does this
> >>> method only work when the states are consistent among these bundles?
> >>>
> >>
> >> My original idea was also to map closely to the composite state, so I
> don't
> >> have any problem
> >> using an int instead of an enum, though I think the state of composite
> is
> >> an int because a composite
> >> *is* a bundle and enums did not exist at the time the bundle interface
> has
> >> been created.
> >> However, Subsystem do not inherit the Bundle or CompositeBundle
> interface,
> >> so we could decide to
> >> use an enum (because the state is semantically an enum).
>
> Ok I see, I think either way would work. :) I think it is easy to
> code when state is the same as what OSGi already defined.
>
> >> That being said, the question you raise about the constituents state is
> >> important. We need to choose
> >> whether we want to represent the state of the subsystem as an aggregate
> of
> >> the constituents state or not.
> >> That would surely be interesting, because you would know the real state
> of
> >> your subsystem much more
> >> easily. The problem is i'm not sure what would happen if one of the
> >> bundle can't start or is manually
> >> stopped by the user. Would this leave the subsystem in the STARTING
> state
> >> until the start or stop
> >> method is called ?
> Yea I can see state is very complicated for subsystem if it represents
> the composite and constituents' states. That made me wonder if we
> want to provide a state for the subsystem?
>
> Or we only provide the state when all the composite and constituents'
> states are consistent, else we mark the state as inconsistent.
>
Yeah, that's a tough problem. Maybe we should have both somehow.
I guess another problem is if we want to represent the state of blueprint
bundles
for example. Because even if a blueprint bundle is active, it does not mean
the blueprint container has been created successfully ...
>
> >> Two things here. First, I'm not sure we would want to return the
> composite
> >> headers.
> >> First because composite headers can't be localized, and second because i
> >> was thinking
> >> about returning the headers from the subsystem manifest as it was
> >> installed, not of the
> >> underlying composite (which includes computed headers for package and
> >> service policies).
> >> So if we return directly the composite headers, it would surely make
> more
> >> sense to use the
> >> same type, but if we don't, i think using Map is more natural. OSGi is
> >> really the only place
> >> that uses Dictionary instead of Map ;-)
>
> Sorry I overlooked the manifest headers are actually slightly
> different. I am fine here. :-)
>
> >>>
> >>> 8. I'd like the add the following methods to Subsystem.java:
> >>>
> >>> + void uninstall() throws SubsystemException;
> >>> +
> >>> + String getScope();
> >>> +
> >>> + void update() throws SubsystemException;
> >>> +
> >>> + void update(InputStream content) throws SubsystemException;
> >>>
> >>> I think the uninstall(), update() are needed, because the
> >>> SubsystemAdmin.uninstall/update can just delegate the work to the
> >>> actual subsystem. In fact, the SubsustemAdmin cannot really do the
> >>> work, unless we make the composite bundle available out of the
> >>> Subsystem.
> >>>
> >>
> >> I agree to your point from an implementation point of view.
> >> I initially put those methods on the Subsystem, but later moved them
> >> to the SubsystemAdmin. I did that because it leads to a cleaner
> separation
> >> between the actors and the responsibilities. In my mind, the
> >> SubsystemAdmin
> >> is responsible for managing the content of subsystems (installation /
> >> update / creation),
> >> whereas the subsystem does not perform such modifications and the only
> >> methods
> >> available are start() and stop(). I had the exact same objection on
> the
> >> application
> >> api some time ago and Alasdair said the design was more service oriented
> >> rather than
> >> object oriented. And now, I think it makes more sense to do it that
> way.
>
> What do you think of adding method Subsystem.getComposite() ? I think
> that would allow users to easily get hold of the composite bundle. If
> we make the composite bundle id as the unique id of the subsystem, the
> other way to get the composite bundle is to get the id associated with
> the subsystem first, then get the bundle, assuming you have the right
> bundle context. If we can get hold of composite bundle easily from
> Subsystem, I think it is ok to have the uninstall/update methods only
> in SubsystemAdmin.
>
Yeah, I don't have any problem to add a getComposite() method.
But in this case, I'm not sure if we need the getConstituents() anymore,
because
this mostly map to
getComposite().getSystemBundleContext().getBundles()
>
> >>>
> >>> I am also attaching a patch for stuff described above. I'd love to
> >>> help on implementing these APIs if that is ok with you.
> >>>
> >>
> >> Do you mind discussing the changes a bit more ?
> >>
> I didn't mean to commit my patch directly... :) I attached a patch
> only to show what I was describing. Whenever the APIs get a bit
> stable, I'd like to help code the implementation of the subsystem APIs
> if that is ok.
>
Sure, that would be much appreciated.
>
> Thanks
>
> Lin
>
--
Cheers,
Guillaume Nodet
------------------------
Blog: http://gnodet.blogspot.com/
------------------------
Open Source SOA
http://fusesource.com