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

Major upgrade and cleanup #2

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

spauny
Copy link

@spauny spauny commented Feb 19, 2020

Did a major cleanup of the plugin and added support for sequential deployment with delay when the server has multiple nodes, among other improvements.
Feel free to review, QA and merge if you want to.

@spauny spauny requested a review from ihorman February 20, 2020 17:16
@vovkozt
Copy link
Collaborator

vovkozt commented Feb 24, 2020

Hello, Iulian

I'm done code review of your changes. Please see summary below:

The following functionality is missing

  • authorization by login/password
  • artifact registration in Deployment Manager in Jelastic dashboard
  • proxy settings usage for network

The following functionality doesn't work properly

  • 'password' parameter is used as 'apiToken'
  • 'api_hoster' is renamed to 'apiHost' (needs to leave old variable also to support backward compatibility)
  • http requests are used while we use https
  • last changes (from JE-51840 Add deploy optional parameters) are not included (no need for separate 'delay' parameter)
  • 'isSequential' parameter is redundant and it is not using directly by deploy service method (sequential deployment apply if is 'delay' parameter sets to any positive value)

We would also suggest to fix the following:

  • our policy is to use our public client lib instead of custom one (I mean com.lindar:well-rested-client), we have plans to exclude apache http client from plugin also
  • we do not use lombok for our projects

spauny and others added 4 commits February 25, 2020 09:10
* Allow user to pass friendly name for node group

This commit allows the user to pass in the friendly name for the node
group (which is displayed in the Jelastic UI) instead of the name, which
can only be fetched via the API.

If an invalid node group is passed (that is, the value passed doesn't
match any node group name or friendly name), then the plugin will error
out.

* add new display name property

* Support both nodeGroup and nodeGroupName

This commit:
- add supports for nodeGroup and nodeGroupName. The value of nodeGroup
  has priority on nodeGroupName and will not be validated.
- add error handling to the validation logic of the nodeGroupName.
- updates the Jelastic API library to the latest version

* upgrade version

Co-authored-by: Iulian <[email protected]>
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.

5 participants