• 
      

    Remove LicenseInfo.private_data and help extensibility.

    Review Request #14496 — Created June 29, 2025 and submitted

    Information

    Review Board
    release-7.1.x

    Reviewers

    Originally when the new licensing code was added, the intent was to back
    license representations by a LicenseInfo and to allow license
    providers to extend that by plugging state into a private_data member.

    Since the original design, support for LicenseInfo subclasses was
    added, largely removing the need for private_data. In practice,
    though, the LicenseInfo dataclass could not be extended with new
    required arguments due to the way that the dataclass was registered.
    By default, dataclasses register fields as positional arguments instead
    of keyword-only arguments, and this means that any field registered
    after the last field with a default must itself have a default, even in
    subclasses. This isn't ideal for this purpose.

    We can't use kw_only=True, since that's only available on Python 3.10
    and higher, and there's no getting around the internal logic that
    impacts subclasses. Plus, dataclasses are slow and truly necessary, just
    a convenience.

    So to help facilitate License Provider backends with custom state, and
    avoid the dataclass issues, this has been converted to a standard class.
    This will also make it easier for subclasses to override it in a
    consistent way without having to tie into the dataclass-specific
    post-construction hooks.

    Unit tests pass.

    Successfully created and used subclasses of LicenseInfo with new
    required fields.

    Summary ID
    Remove LicenseInfo.private_data and help extensibility.
    Originally when the new licensing code was added, the intent was to back license representations by a `LicenseInfo` and to allow license providers to extend that by plugging state into a `private_data` member. Since the original design, support for `LicenseInfo` subclasses was added, largely removing the need for `private_data`. In practice, though, the `LicenseInfo` dataclass could not be extended with new required arguments due to the way that the dataclass was registered. By default, dataclasses register fields as positional arguments instead of keyword-only arguments, and this means that any field registered after the last field with a default must itself have a default, even in subclasses. This isn't ideal for this purpose. We can't use `kw_only=True`, since that's only available on Python 3.10 and higher, and there's no getting around the internal logic that impacts subclasses. Plus, dataclasses are slow and truly necessary, just a convenience. So to help facilitate License Provider backends with custom state, and avoid the dataclass issues, this has been converted to a standard class. This will also make it easier for subclasses to override it in a consistent way without having to tie into the dataclass-specific post-construction hooks.
    9982f25aa7714bd7119795589848679e127073e2
    Description From Last Updated

    I thought we can't use this on Python 3.8 or 3.9?

    daviddavid
    david
    1. 
        
    2. reviewboard/licensing/license.py (Diff revision 1)
       
       
      Show all issues

      I thought we can't use this on Python 3.8 or 3.9?

      1. We can't. I realized this after putting it up yesterday, and went off in search of the best solution here.

        Tried backporting the 3.13 version of dataclasses, which has this and a bunch more features. But that uses match syntax.

        Tried 3.10, which is the minimum with kw_only. That had other issues that weren't worth pursuing.

        Ultimately I decided to just rewrite this as a normal class, but called it a night before I finished. That'll be up today.

        Fun fact: dataclasses turn out to be about 33x slower to define or subclass than a normal equivalent class. Not a big deal unless doing it dynamically and a lot, but I feel less good about sprinkling them everywhere. Curious how Django and Pydantic models compare.

    3. 
        
    chipx86
    david
    1. Ship It!
    2. 
        
    maubin
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-7.1.x (0c2a5bd)