Skip to content

compute: API for server actions #67

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.

Sign up for GitHub

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

Jump to bottom
Merged
olivergondza merged 4 commits into from
Aug 27, 2020
Merged

compute: API for server actions #67

olivergondza merged 4 commits into from
Aug 27, 2020

Conversation

@mrsujitsah
Copy link
Contributor

@mrsujitsah mrsujitsah commented Jul 18, 2020

I have written the API for implementation of server event actions. This API fetch sever event actions

@mrsujitsah
Copy link
Contributor Author

mrsujitsah commented Jul 20, 2020

@olivergondza please review

olivergondza
olivergondza requested changes Aug 8, 2020
View reviewed changes
Copy link
Member

@olivergondza olivergondza left a comment

Choose a reason for hiding this comment

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

Sorry I am getting to this with delay. IIUC, this is new api for https://docs.openstack.org/api-ref/compute/index.html#servers-actions-servers-os-instance-actions.

It seems all my comments are related to terminology where I would like to stick with the naming of the API for easier understanding.

olivergondza
olivergondza reviewed Aug 10, 2020
View reviewed changes
core/src/main/java/org/openstack4j/model/compute/ServerAction.java Outdated
import org.openstack4j.model.common.ServerActionEvent;

/**
* This interface describes the getter-methods (and thus components) of a Event.
Copy link
Member

@olivergondza olivergondza Aug 10, 2020

Choose a reason for hiding this comment

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

It is ServerAction now. Better to refer to symbols using {@link ...} to catch these errors.

@olivergondza
Copy link
Member

olivergondza commented Aug 10, 2020

I see the naming is easier to understand now. Thanks!

However, I would prefer to do these to make it internally consistent:

EventServices -> ServerActionsService
EventServicesImpl -> NovaServerActionsService
EventListBuilder -> ServerActionEventBuilder
GenericEventsList -> NovaServerActionEvent
GenericEventsList.EventConcreteBuilder -> NovaServerActionEvent.NovaServerActionEventBuilder (or just NovaServerActionEvent.Builder)
ServerEvent -> NovaServerAction

Also, the package org.openstack4j.model.common is for the model objects shared amongs services - which is not the case for any of the classes added. It either belongs to org.openstack4j.openstack.compute.domain or org.openstack4j.model.compute.

@olivergondza olivergondza merged commit 9a0d006 into openstack4j:master Aug 27, 2020
@olivergondza
Copy link
Member

olivergondza commented Aug 27, 2020

Merged and amended with 019a63a. Thanks!

@olivergondza olivergondza mentioned this pull request Aug 27, 2020
Closed
@olivergondza olivergondza changed the title API for server event actions compute: API for server actions Sep 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@olivergondza olivergondza Awaiting requested review from olivergondza

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mrsujitsah @olivergondza