[ovirt-devel] new internal stable modules + proposal

[Martin Polednik]

On 29/03/16 20:36 +0300, Nir Soffer wrote:
>On Tue, Mar 29, 2016 at 7:44 PM, Martin Polednik <mpolednik at redhat.com> wrote:
>> On 29/03/16 11:12 -0400, Francesco Romani wrote:
>>>
>>> Hi,
>>>
>>> in the last Vdsm developer call we agreed to promote a few modules in the
>>> common repository.
>>> The common repository provides the additional guarantees over regular
>>> modules in lib/vdsm/
>>>
>>> - stable API
>>> - (thus) safe to use across verticals
>>>
>>> the planned moves are:
>>>
>>> lib/vdsm/schedule.py -> lib/vdsm/common/schedule.py
>>> lib/vdsm/periodic.py -> lib/vdsm/common/periodic.py
>>> lib/vdsm/virt/api.py -> lib/vdsm/common/api.py
>>>
>>> Question is if those modules should go under common/ or under another
>>> subdirectory, maybe infra?
>>
>>
>> Hi.
>>
>> I agree that some modules should be kept (and advertised) as stable.
>>
>> The name 'common' is more suited for such package as 'infra' is very ovirt
>> specific and could hurt readability and navigation of the project. It does
>> make sense to structure the project as follows:
>>
>> lib/vdsm/common
>> lib/vdsm/virt/common
>> lib/vdsm/storage/common
>> lib/vdsm/network/common
>
>This creates a dependency between the packages. The goal is to have
>packages that do
>not depend on each other except the common package, which is library
>code used by
>other parts.

True, forgot about that. We fortunately still have possibility of further
sharding 'common' if that need arises.

>> Taking extra care taken to avoid 'common junkyard' situation and
>> making sure that the scope of common corresponds to it's package.
>>
>> List of modules that I consider public (mostly author/main contributor
>> of these lately):
>>
>> lib/vdsm/cpuarch.py
>> lib/vdsm/cpuinfo.py
>> lib/vdsm/hostdev.py
>> lib/vdsm/machinetype.py
>> lib/vdsm/numa.py
>> lib/vdsm/osinfo.py
>> lib/vdsm/udevadm.py
>
>+1
>
>>> Lastly, i have a proposal about better handling of those modules.
>>>
>>> First, the mere fact a module is placed under lib/vdsm/common provides the
>>> extra guarantees I mentioned.
>>> But should we added more annotations?
>>>
>>> for example something like
>>>
>>> __API__ = {}
>>>
>>> near the top of the module
>>>
>>> if this attribute exist, then the module is safe to use across verticals,
>>> has stable API and so forth
>>> (this is _in addition_ to the common/ package, not as replacement).
>>>
>>> Like:
>>>
>>> __API__ = {
>>>  "introduced-in": "4.14.0",
>>>  "deprecated-from": "4.18.0",
>>>  "removed-at": "4.20.0",
>>>  "contact": "fromani at redhat.com"
>>> }
>>>
>>> We could refine further this concept if we like it. The idea is to be
>>> lightweight as possible while
>>> carrying all the information we need.
>>
>>
>> I agree about keeping global metadata in that way, except ideally
>> using docstrings for better readability (e.g. interactive help()). To
>> better reflect the needs for granular deprecation and based on our
>> private discussion on this subject, I'd like to see at least
>> @deprecated(target_removal_version) decorator. On top of that, we need
>> properly documented public API and deprecation process.
>
>Using standard way such as __author__ is better than docstrings, see for example
>the output of help(module)

Partially agreed. Having proper docstrings wouldn't hurt either. I
frequently use repl w/ vdsm now that it is in lib, this would help
myself - am I the only one?

I would still like to see some kind of proper deprecation in private
APIs. It might as well serve as a testing ground for deprecation in
public API.

>Nir
>_______________________________________________
>Devel mailing list
>Devel at ovirt.org
>http://lists.ovirt.org/mailman/listinfo/devel