[ovirt-devel] new internal stable modules + proposal

[Nir Soffer]

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.

> 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)

Nir