Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature] Added missing endpoints for DeviceFirmware and UpgradeOperation #206 #231

Merged
merged 12 commits into from
May 29, 2023

Conversation

Aryamanz29
Copy link
Member

@Aryamanz29 Aryamanz29 commented May 4, 2023

Blockers

  1. upgrade-operation/

    • List all the upgrade operation in the system
  2. upgrade-operation/<id>/

    • Display detail of a particular upgrade id
  3. device/<device_pk>/upgrade-operation/

    • return a list of upgrade-operation ids for a given device id
  4. device/<device_pk>/firmware/

    • PUT: if DeviceFirmware doesn't exist, create a new object else
      update the image of the DeviceFirmware.
      Returns the DeviceFirmware object and if an upgrade operation
      was created, also return the id of the UpgradeOperation.

    • GET: return the DeviceFirmware object,

  • Added endpoints tests.
  • Updated documentation.

Closes #206

tspare and others added 6 commits May 4, 2023 15:17
The code adds four endpoints:

1.  upgrade-operation/
    - List all the upgrade operation in the system
2.  upgrade-operation/<id>/
    - Display detail of a particular upgrade id
3.  device/<id>/upgrade-operation/
    - return a list of upgrade-operation ids for a given device id
4.  device/<id>/firmware/
    - PUT: if DeviceFirmware doesn't exist, create a new object else
            update the image of the DeviceFirmware.  Returns
            the DeviceFirmware object and if an upgrade operation
            was created, also return the id of the UpgradeOperation
    - GET: return the DeviceFirmware object

Closes #206
@Aryamanz29 Aryamanz29 added the enhancement New feature or request label May 4, 2023
@Aryamanz29 Aryamanz29 self-assigned this May 4, 2023
@Aryamanz29 Aryamanz29 marked this pull request as ready for review May 4, 2023 11:36
@Aryamanz29 Aryamanz29 requested a review from nemesifier May 4, 2023 11:36
@Aryamanz29 Aryamanz29 force-pushed the issue-206/add-missing-endpoints branch 2 times, most recently from ff245c8 to 9a26cb0 Compare May 4, 2023 12:02
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I will spend more time testing this tomorrow!

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's some feedback from the first round of testing.
I did a quick test of the different endpoints which seem to work quite well.
Below I have some minor change requests.

Upgrade Operation List:

  • can you allow to filter by org and org_slug?
  • can you allow to filter by device ID (can we avoid generating the select with the entire device list in the UI to avoid this to fail on large instances?)
  • allow filter by image ID (can we avoid generating the select with the entire image list in the UI to avoid this to fail on large instances?)
  • allow filter by status

Device Upgrade Operation List:

  • allow filter by status

I will do another round of acid testing and code review tomorrow.

README.rst Outdated Show resolved Hide resolved
- Additionally, included the missing mentions
of API list filters for the following endpoints:
build/, build/<build_id>/image, and batch-upgrade-operation/
@Aryamanz29 Aryamanz29 force-pushed the issue-206/add-missing-endpoints branch from 5e8256a to 86a3e8e Compare May 23, 2023 09:17
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's looking good, I have some remaining doubts below.

openwisp_firmware_upgrader/base/models.py Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
- `ValidatedModelSerializer` is not required for read-only API views.
@Aryamanz29 Aryamanz29 force-pushed the issue-206/add-missing-endpoints branch from f922903 to 54f8d6a Compare May 25, 2023 10:15
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 👍

@nemesifier nemesifier merged commit 3b3eaab into master May 29, 2023
8 checks passed
@nemesifier nemesifier deleted the issue-206/add-missing-endpoints branch May 29, 2023 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature] REST API is missing endpoints for DeviceFirmware
3 participants