launchpad-dev team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Hi

I'd like some input with an issue concerning collections in launchpadlib.

The background is that I want to have launchpadlib recognise a new top
level collection called 'services'. Services are looked up by name. The
collection is heterogeneous in that each named service provides
different capabilities via different exported methods. All services
extend an IService interface and this is how the collection is currently
defined:

class IServiceFactory(Interface):

    export_as_webservice_collection(IService)

    @operation_parameters(name=TextLine(required=True))
    @operation_returns_entry(IService)
    @export_read_operation()
    @operation_for_version("beta")
    def getByName(name):
        """Lookup a service by name."""

I want to allow launchpadlib to be used like so:

lp = Launchpad(....)
service = lp.services['myservice']
service.foo()

(As an aside, the current syntax required to access a named service is:

    # XXX 2012-02-23 wallyworld bug 681767
    # Launchpadlib can't do relative url's
    service = launchpad.load(
        '%s/+services/sharing' % self.launchpad._root_uri)

Even if bug 681767 were fixed, the client would still need to know the
URL to traverse whereas having a named 'services' collection hides this
implementation detail. Perhaps this is ok though?)

Because the services collection is heterogeneous, there needs to be some
way which allows launchpadlib to know what each service instance in the
collection provides. My initial solution was to define a new ServiceSet
in launchpadlib (a solution already used for projects, bugs,
distributions etc):

class ServiceSet(CollectionWithKeyBasedLookup):
    """A custom subclass capable of service lookup by service name."""

    def _get_url_from_id(self, key):
        """Transform a service name into the URL to a service."""
        return str(
            self._root._root_uri.ensureSlash()) +
                '+services/' + str(key)

    # All services are different so we need to ensure each service
    # representation is retrieved when needed.
    collection_of = None

This works well and correctly deals with the fact that each named
service instance has different capabilities. However, concerns were
raised that it adds to LOC count and could be done generically without
the helper class.

My understanding is that an alternative solution would require changes
and/or enhancements to the WADL generation rules, and for additional
information to be added to the WADL. At the moment, lazr.restful (which
is used by launchpadlib) doesn't really know what constitutes a top
level collection and uses heuristics to determine this.
eg from ServiceRootResource

    # XXX sinzui 2008-09-29 bug=276079:
    # Top-level collections need a marker interface
    # so that so top-level utilities are explicit.
    if (provided.isOrExtends(ICollection)
         and ICollection.implementedBy(registration.factory)):

This issue would need to be addressed along with the ability to define
more specifically what's in each collection.

I would love to get input from the smart folks on this list as to what
the best way forward is. I think Gary, Francis, (and Leonard) have
worked on this specific stuff in the past. Do we want to attempt to
tackle the higher level issues highlighted above? Apart from the bug
concerning the definition of top level collections (276079), I couldn't
find a bug specifically related to heterogeneous collections. The LOC
count for an alternative solution would be (far?) greater than what my
solution has, but may allow launchpadlib to in the future handle new
heterogeneous collections without additional code changes. I am wary of
diverting too much time away from the disclosure project for a nice to
have but non-core disclosure item. I see the options as:

1. Adopt my current solution which defines a ServiceSet in launchpadlib
and allows the syntax lp.services['myservice']
2. Fix bug 681767 (not sure of the effort required) and use syntax
lp.load('/+services/myservice')
3. Address the WADL/lazr.restful issues to support syntax
lp.services['myservice'] and make launchpadlib 'futureproof'

Thoughts?