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

EAM API: fixes for app instance post (alternative to #316) #333

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gainsley
Copy link
Collaborator

@gainsley gainsley commented Feb 3, 2025

What type of PR is this?

  • correction
  • enhancement/feature
  • documentation

What this PR does / why we need it:

  • Clarifies the expected behavior when deploying multiple app instances via the appinstances POST API
  • Generalizes the behavior of the API as a "bulk" API, which is just multiple of what would have been a singular API
  • Assign a name to the AppInstance. Without this we cannot have declarative/infrastructure-as-code clients layered on top of the NBI API. See EAM: Suggest to add App Instance Name #255. Also makes it consistent with other objects that also have names, i.e. Apps, EdgeCloudZones.
  • Adds more necessary information to the returned AppInstanceInfo, including the AppId (previously no way to figure out which App the AppInstance was an instance of). Makes the necessary information required. Changed the EdgeCloudZoneName to an EdgeCloudZoneId for consistency, as we always reference other objects by their ID, not their name.
  • Adds a status code and message to each deploy response to allow for distinguishing the results of each deployment request separately.

Note that this is an alternative approach to #316 which attempted to address the same issues but using a singular API approach. This PR is the bulk API approach which more closely resembles the original intent of the API.

Which issue(s) this PR fixes:

Fixes #256, #255

Special notes for reviewers:

One potential contentious change I have made is to include the AppId with each AppInstanceDeployRequest, rather than make it common across all requests. The primary reason was to generalize the handling of "bulk" APIs, such that a bulk API simply has request/response objects which are arrays of what the singular API would be. This approach could be extended to other APIs (although I don't really see any candidates at the moment). This also makes it more flexible, allowing a user to deploy different AppIds across various edge-sites in a single API call, given that the intent here is to avoid the overhead of making multiple API calls to deploy app instances.

Changelog input

 release-note
- clarify and fix behavior of appinstances POST

Additional documentation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EAM: Suggest that POST appinstance targets a single EdgeCloudZone
1 participant