The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/pylxd/pull/283
This e-mail was sent by the LXC bot, direct replies will not reach the author unless they happen to be subscribed to this list. === Description (from pull-request) === This is a proposal/spec to explain what a v3 of the pylxd API would be like and why it is needed. This would be a breaking change for clients of pylxd. It's opened for discussion here; thoughts both for and against are welcomed.
From 10638ac20775f6acef741a6db24ad2a423b9cf80 Mon Sep 17 00:00:00 2001 From: Alex Kavanagh <a...@ajkavanagh.co.uk> Date: Wed, 7 Mar 2018 15:49:07 +0100 Subject: [PATCH] Add proposal for version 3.0 of the pylxd API This is a proposal/spec to explain what a v3 of the pylxd API would be like and why it is needed. This would be a breaking change for clients of pylxd. --- MANIFEST.in | 3 +- notes/version-3-proposal.rst | 326 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 328 insertions(+), 1 deletion(-) create mode 100644 notes/version-3-proposal.rst diff --git a/MANIFEST.in b/MANIFEST.in index 90f8a7a..2111a6a 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -2,5 +2,6 @@ include AUTHORS include ChangeLog exclude .gitignore exclude .gitreview +exclude notes -global-exclude *.pyc \ No newline at end of file +global-exclude *.pyc diff --git a/notes/version-3-proposal.rst b/notes/version-3-proposal.rst new file mode 100644 index 0000000..7d6af71 --- /dev/null +++ b/notes/version-3-proposal.rst @@ -0,0 +1,326 @@ +.. + Copyright 2018 Canonical Limited + + This work is licensed under a Creative Commons Attribution 3.0 + Unported License. + http://creativecommons.org/licenses/by/3.0/legalcode + +.. + This template should be in ReSTructured text. Please do not delete + any of the sections in this template. If you have nothing to say + for a whole section, just write: "None". For help with syntax, see + http://sphinx-doc.org/rest.html To test out your formatting, see + http://www.tele3.cz/jbar/rest/rest.html + +=========================================== +pyLxd 3.0 Proposal - backwards incompatible +=========================================== + +pylxd 2.x has an odd interface and tends not to be very flexible to +changes/variations in the parent LXD API. The 'oddness' in the interface is +that the *api* and *client* are on every instance, and the raw API methods tend +to have to be used inconsistently. + +Problem Description +=================== + +A detailed description of the problem. + +The pylxd client API has the following issues/inconsistencies, which should be +addressed to give a more natural Pythonic interface to lxd, which is the +purpose of the library. + +1. The attribute/manager/__slots__ system, whilst very clever, is inflexible + and simply discards new attributes that are added to (point) releases of the + LXD package. + +2. The attribute system only tries to do a single level of JSON loading, and + doesn't load the whole tree of metadata returned in an API call. Thus you + get the (slightly) perverse situation of some attributes container JSON + encoded strings, rather than a dictionary/list object. See `#214 + <https://github.com/lxc/pylxd/issues/214>`_ for example. + +3. Each ``object`` returned has hard-coded methods for ``.api``, ``.client`` + and ``.save`` which allows/encourages fairly strange code structures. e.g. + if you have a *container* object, you can (oddly) do + ``container.client.profiles.all()``. This also means that an object can + never have an attribute called *client*, *api* or *save*. + +4. The object system is only partially done. e.g. The *container* object has + *profiles* as simple list attribute; it really ought to be profile objects. + So code such as ``container.profiles['general']`` should return the general + profile (or a ``KeyError``), and ``container.profiles`` ought to be a + dictionary like object. + +5. There is a surprising result that if you ask for the same container twice, + they are two different objects. This means that you can change them + independently in different ways, and then ``.save()`` then they would + overwrite it each in interesting ways (last write wins). It would be better + to serve up the same object each time it is fetched. This can be acheived + with reference counting and weak-refs. + +6. nova-lxd will need a some changes to the API usage to match this work. This + is not a particularly difficult task. + +Proposed Change +=============== + +Here is where you cover the change you propose to make in detail. How do you +propose to solve this problem? + +If this is one part of a larger effort make it clear where this piece ends. In +other words, what's the scope of this effort? + +The proposal is to release a pylxd 3.0 that is backwards *incompatible* with +the 2.0 series. This will have a modified external API that is more Pythonic +and also fixes some of the unexpected quirks of the existing external API. +These are in detail: + +**1. Resolve inflexible __slots__ arrangement** + +The issue with the ``__slots__`` is that it fixes what attributes are loaded +from the ``metadata`` key returned during a ``GET`` call. i.e. new attributes, +which do appear from time to time, *and are frequently backported with a +feature flag to the stable releases*, aren't accessible via the pylxd library. +This is, frankly, annoying. + +A solution to this problem is to store the whole ``metadata`` JSON object +(after it has been parsed) on an internal var (say ``_metadata``) and +dynamically accept attribute access via key lookups. This would make any new +attributes read-only which is, at least, more useful than not existing. Then +the different lxd object classes would just need lists for what keys are +writable and which ones relate to other objects (the so called *manager* +methods). + +**2. Single level of JSON loading** + +This one is very easy to fix. Instead of just assigning each top-level key to +a local attribute, just JSON parse the entire ``metadata`` section and store +it. This will parse (and thus, JSON validate) the attributes. They can then +be lazily accessed via ``__getattribute__()`` if they exist in the +``_metadata``. + +**3. .client, .api properties on every lxd object** + +The end result will be that every attribute on an lxd object will have a direct +mapping to the (new) ``_metadata`` private attribute on the object. i.e. the +``class Model`` will have a private ``_metadata`` member that is the +``metadata`` returned by API calls to lxd. + +The ``.save()``, ``.dirty()``, ``.sync()``, ``.rollback()``, ``.delete()`` and +``.marshal()`` methods will be removed completely, and new functions will be +added to the top level pylxd module to access their functionality. Therefore, +saving an lxd object will then take the form: + +.. code-block:: python + + import pylxd + + ... + pylxd.save(some_object) + +The concept is that the objects themselves become just data entities, but will +a little bit of behaviour around read-only attributes, and 'manager' attributes +for 'chained' objects (e.g. profiles on a container). + +This ``client`` and ``api`` properties will be renamed to ``_client`` and +``_api`` to make them 'private' and thus, for internal library use; i.e. +signalling that the user of the library is not supposed to use these methods on +the objects. If possible, they may be removed entirely, although it's not +clear how dependent objects could be automatically fetched without having at +least the ``_client`` available on each object. + +Note, that currently, the ``.api`` property is set for each object to resolve +the API path and instead having an ``_url`` property which returns the API +endpoint url fragment for that container. Then the ``save(...)``, +``sync(...)`` can use that as a data string, rather than as a convoluted client +accessing property which is hard to follow when reading the code. + +**4. The object system is only partially done** + +e.g. Container objects have profiles, but they just return strings. i.e. to +access a profile, currently: + +.. code-block:: python + + import pylxd + + c = pylxd.Client() + container = c.containers.get('test') + profile = c.profiles.get(container.profiles) + +This would be much more natural if it could be accessed as: + +.. code-block:: python + + import pylxd + + c = pylxd.Client() + profile = c.containers['test'].profiles + +and the ``profile`` object would be a pylxd ``Model()`` object, where the +``used_by`` attribute would be a dictionary like object which had 'test' as a +key, and the test container as a value. e.g. + +.. code-block:: python + + container = c.profiles['default'].used_by + <... Manager for containers ...> + +i.e. the ``used_by`` property would return a manager to access profiles. + +**5. Make the manager classes 'smarter' and self documenting** + +The Manager classes for various 'lists' of lxd objects could be made smarter, +and thus more intuitive to use. At the moment, they have ``.get()`` and +``.all()`` methods (as well as the other class methods) of the managed object +class. (e.g. Container has a class method of ``create``). + +However, the code would be eaiser to understand if the Manager classes had the +class methods for the various objects, without having to dynamically set them, +and the object classes were 'dumber'. In the process, it is possible to make +the classes easier to use. + +So if the manager class supports the ``__getitem__()`` method then we can use +dictionary-like indexing to provide a natural interface to accessing +containers. The two specific behaviours the manager classes should have are +dictionary getitem behaviour and iterator behaviour. This means that they can +be used like dictionaries and also as iterators (and thus 'listable'). + +So examples: + +.. code-block:: python + + c = pylxd.Client() + list(c.containers) + [<container... 1>, <container... 2>, etc.] + + c.containers['test'] # access the test container. + c.containers # access the fourth container (0 indexed) + c.containers.create(...) # create a container + c.containers.delete(str or container) # delete a container + pylxd.delete(container) + pylxd.save(container) + +**6. Raw API access** + +The pythonic interface is very useful, but sometimes, just accessing the raw +methods and returning the response is useful. pylxd provides this via the +``.api`` property, but it is missing the ``patch()`` method. Also, with the +other changes, the ``api`` property on each lxd object will (probably) +disappear, and thus a little work may be necessary to be able to keep doing +something like: + +.. code-block:: python + + c = pylxd.Client() + response = c.api.containers['test'].get() + +It may be better to provide a separate *raw* api interface that can be used to +generate the URL that will then be used. e.g. + +.. code-block:: python + + c = pylxd.Client() + url = c.url.containers['test'].files['/abc/def'] + file = c.raw.get(url) + +It's not clear what the best approach to this is yet. + +**7. Caching of objects** + +This is the principal that if the system hands out the same url object twice, +it ought to be the same object. This (optional) part of the proposal is to +weakref count the objects by URL and give out the same object if the weakref is +still valid (by url). Note that this is not the same as being *thread safe* +which the library isn't at present (due to the way that multiple objects of the +same resource are handed out). This would allow the API to be used in a way +that minimises the API calls, yet allows it to be used effectively. + +**Self Documenting code** + +At present there are separate ReST files for each of the ManagerModel classes +*because* they can't be documentd on the *actual* classes (the methods are +runtime imported from the managed class). With the changes suggested in this +proposal, the Manager classes can be properly documented in ``docstrings`` +which would help to ensure that documentation stays up to date with code +changes. + +Thus, the key to 'self-documenting' is that the documentation on how to the +methods on the Manager class can actually *be* in the manager class, along with +type information, etc., and this can be used to generate the API documentation. +This would make for more maintainable documentation for the library. + +Alternatives +------------ + +None + +Implementation +============== + +Assignee(s) +----------- + +Who is leading the writing of the code? Or is this a blueprint where you're +throwing it out there to see who picks it up? + +If more than one person is working on the implementation, please designate the +primary author and contact. + +Primary assignee: + <launchpad-id or None> + +Can optionally list additional ids if they intend on doing substantial +implementation work on this blueprint. + +TBC + +Gerrit Topic +------------ + +NA - The code is managed in GitHub. A separate 3.0 development branch will be +created and code/documentation merged using the GitHub Pull Request system. + +Work Items +---------- + +Work items or tasks -- break the feature up into the things that need to be +done to implement it. Those parts might end up being done by different people, +but we're mostly trying to understand the timeline for implementation. + +TBC + +Repositories +------------ + +No new repositories are needed. + +Documentation +------------- + +The existing documentation will need to be changed. This documentation is +already generated as a sphinx job with the python module. It is proposed as +part of the work to migrate the separately maintained written documentation +pages about the containers, profiles, (etc.) and move them to the ManagerModel +doctstring documentation. This would enhance the documentation and also ensure +that it stays up to date with changes in the library. At present, because the +documentation is separate from the code, it often ages and doesn't correspond +to the actual API. + +Security +-------- + +No change to security is envisaged. + +Testing +------- + +The existing test infrastructure is sufficient to adequately test the changes. +The unit tests will need to be changed to accommodate the API changes. + +Dependencies +============ + +- nova-lxd will be impacted by the API change, but the changes will just be how + the pylxd API is called, rather than semantic changes to how the API is used.
_______________________________________________ lxc-devel mailing list email@example.com http://lists.linuxcontainers.org/listinfo/lxc-devel