Merge lp://staging/~danilo/maas/django-upgrade-licensekeyform-immutable-querydict into lp://staging/~maas-committers/maas/trunk

Proposed by Данило Шеган
Status: Merged
Approved by: Blake Rouse
Approved revision: no longer in the source branch.
Merged at revision: 6105
Proposed branch: lp://staging/~danilo/maas/django-upgrade-licensekeyform-immutable-querydict
Merge into: lp://staging/~maas-committers/maas/trunk
Prerequisite: lp://staging/~danilo/maas/django-upgrade-immutable-querydict
Diff against target: 200 lines (+73/-48)
5 files modified
src/maasserver/api/license_keys.py (+17/-2)
src/maasserver/api/tests/test_licensekey.py (+38/-0)
src/maasserver/forms/__init__.py (+0/-23)
src/maasserver/forms/tests/test_licensekey.py (+1/-16)
src/maasserver/views/tests/test_settings_license_keys.py (+17/-7)
To merge this branch: bzr merge lp://staging/~danilo/maas/django-upgrade-licensekeyform-immutable-querydict
Reviewer Review Type Date Requested Status
Blake Rouse (community) Approve
Review via email: mp+326079@code.staging.launchpad.net

Commit message

Move osystem/distro_series combining logic to API handlers instead of LicenseKeyForm.full_clean() which now always requires such a form for distro_series field.

Description of the change

(follow-up from https://code.launchpad.net/~danilo/maas/django-upgrade-immutable-querydict/+merge/325995)

LicenseKeyForm fields are set up dynamically, and distro_series field is set up so that the "keys" are in the form os/series. With Django 1.11, POST data has become immutable in full_clean(), so to avoid the hack of setting internal _mutable to True, this fixes the logic so the combining of osystem and distro_series happens in the API layer instead.

And contrary to what the comment in full_clean() says, it seems API never really supported specifying only distro_series in the os/series format (a test I added for that actually failed with the old code): now it does (see test instructions below).

Testing instructions:

1. make syncdb && make sampledata && make bin/maas
2. make run
3. bin/maas login dev-1.8 http://...:5240/MAAS (use admin API key)
4. Upload a Windows image using a release that requires a license key:

  $ bin/maas dev-1.8 boot-resources create name=windows/win2012 architecture=amd64/generic filetype=ddtgz content@=/etc/passwd
5a. Browse to MAAS settings page, "License keys" section should now show up and you can still add keys ("valid" Windows key is AAAAA-BBBBB-CCCCC-DDDDD-EEEEE).

    Note that when license key validation fails, there's nothing shown in the form: I don't believe that's due to my changes, though.

5b. To test via the API, you can do:

$ bin/maas dev-1.8 boot-resources create name=windows/win2016 architecture=amd64/generic filetype=ddtgz content@=/etc/group

# Use only distro_series.
$ bin/maas dev-1.8 license-keys create distro_series=windows/win2016 license_key="AAAAA-BBBBB-11111-22222-33333"Success.
Machine-readable output follows:
{
    "resource_uri": "/MAAS/api/2.0/license-key/windows/win2016",
    "distro_series": "win2016",
    "osystem": "windows",
    "license_key": "AAAAA-BBBBB-11111-22222-33333"
}
$ bin/maas dev-1.8 license-key delete windows win2016

# Use split up osystem and distro_series.
$ bin/maas dev-1.8 license-keys create osystem=windows distro_series=win2016 license_key="AAAAA-BBBBB-11111-22222-33333"
Success.
Machine-readable output follows:
{
    "resource_uri": "/MAAS/api/2.0/license-key/windows/win2016",
    "distro_series": "win2016",
    "osystem": "windows",
    "license_key": "AAAAA-BBBBB-11111-22222-33333"
}

To post a comment you must log in.
Revision history for this message
Данило Шеган (danilo) :
Revision history for this message
Blake Rouse (blake-rouse) wrote :

Looks good. Split out very well and much cleaner then mutating the request.data. Thanks for making this change, even though it was not really required for the work you where doing.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.