Skip to content

ApacheCloudStackClient: Remove assumed client API url suffixes#25

Merged
rafaelweingartner merged 1 commit intoAutonomiccs:masterfrom
yadvr:remove-api-url-assumption
Apr 4, 2017
Merged

ApacheCloudStackClient: Remove assumed client API url suffixes#25
rafaelweingartner merged 1 commit intoAutonomiccs:masterfrom
yadvr:remove-api-url-assumption

Conversation

@yadvr
Copy link
Contributor

@yadvr yadvr commented Apr 4, 2017

This removes assumptions around the CloudStack API urls to have /client/api, or
/api suffix. By removing this, it will be responsibility of the client-library
user to ensure that the passed URL while creating the ApacheCloudStackClient
instance is the API url.

In several production environments, the API url may be behind a reverse-proxy.
This change addresses such environments.

Ping @rafaelweingartner - please review and advise any other changes.

This removes assumptions around the CloudStack API urls to have /client/api, or
/api suffix. By removing this, it will be responsibility of the client-library
user to ensure that the passed URL while creating the ApacheCloudStackClient
instance is the API url.

In several production environments, the API url may be behind a reverse-proxy.
This change addresses such environments.

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
@autonomiccs-jenkins
Copy link
Contributor

Can one of the admins verify this patch?

@rafaelweingartner
Copy link
Contributor

test this please

@yadvr
Copy link
Contributor Author

yadvr commented Apr 4, 2017

@rafaelweingartner how do I test this, or it's a command to run tests?

@rafaelweingartner
Copy link
Contributor

rafaelweingartner commented Apr 4, 2017

@rhtyd may bad, when I said "test this please", it was not for you, it was for Jenkins :)

Well, the code has +85% of test case coverage. So, I am blindly trusting on my Jenkins builds; during the build, I am running all of the test cases. Also, if the project does not build, or if something is not complying with the standards such as licensing headers and others, Jenkins will tell us here. There is the quality Gate of Sonar as well.

For this PR, everything seems to be ok.

@rafaelweingartner
Copy link
Contributor

I have some scenarios where this “adjust” was quite helpful. However, after giving some thought to the proposal, I do think we could take this responsibility out of the library.

Everything is green here, so I am going to merge it. I will not close a new version right now because I think there might be some other changes to come.

@rafaelweingartner rafaelweingartner merged commit d939ac7 into Autonomiccs:master Apr 4, 2017
@yadvr
Copy link
Contributor Author

yadvr commented Apr 5, 2017

Thanks @rafaelweingartner

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.

3 participants