erikbocks commented on PR #12502: URL: https://github.com/apache/cloudstack/pull/12502#issuecomment-3804729385
Hello, @DaanHoogland and @sureshanaparti Firstly, thank you for the reviews and for the suggestions. Regarding some the questions brought by @sureshanaparti: > What event description is logged for create/add calls (UUID might not exist)? This behavior will depend on how the API handles its events' creation. The majority of APIs uses the `ActionEvent` annotation, which stores information as the description and the event type. This information is gathered by some `intercept...` methods present at the `ActionEventInterception` class. These methods gather the information defined at the method's annotation, and publish the event. Some APIs, as the asynchronous ones, have some intermediate events informing whether the operation was `Scheduled` or `Started`. The description of the `Scheduled` events are based on the `getEventDescription()` method, which is the main focus of this PR. Events with other states, as `Completed` and `Created` ones, use the `CallContext.eventDetails` attribute. This attribute is set during the API processing, and is used together with the `ActionEvent` description. The majority of APIs for creation or addition of resources does not have any events informing the resource's UUID; thus, the change in this PR does not affect them. However, there are some APIs that have an event description that display a UUID. These APIs are the ones that inherit from the `BaseAsyncCreateCmd` class, and thus, have the `create()` method, which is a method that is executed prior to the `BaseCmd`'s `execute()`. This means that, when the API processing flow reaches the event's description obtention step, the resource was already created. > can add to Map here, instead of querying db again. The reason for not inserting the UUID obtention logic there is because these lines are only executed when the parameter has the `@ACL` annotation. In order to address both parameters that has these annotation, as the ones that does not, the UUID obtention logic was added to the `translateUuidToInternalId()` method. This method is executed inside the `setFieldValue()` method, which is executed at line 210, prior to the referenced snippets (lines 264 and 287). In addition to that, is important to note that the second snippet refers to the processing of a UUID list parameter. As changing the processing of `List` type parameters would require changing the current logic, in order to allow the storing of the resources' UUIDs, I choose not to address these cases in this PR. --- @DaanHoogland, after analysing @sureshanaparti's reviews, I noticed that some events details set in the `CallContext` class were not being addressed. Therefore, I went through these occurrences and made changes to them, in order to use the new UUID obtention method. Could you please review the files again? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
