Skip to content
This repository has been archived by the owner on Jul 25, 2020. It is now read-only.

Add vSphere support #61

Closed
wants to merge 72 commits into from
Closed

Add vSphere support #61

wants to merge 72 commits into from

Conversation

igreenfield
Copy link

No description provided.

@buildhive
Copy link

jclouds » jclouds-labs #1029 FAILURE
Looks like there's a problem with this pull request
(what's this?)

@@ -0,0 +1,132 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
Copyright 2014 Cisco Systems, Inc
Copy link
Member

Choose a reason for hiding this comment

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

As per the ASF guidelines, there cannot be a copyright statement here, as far as I understand. Would you be able to remove this, @igreenfield?

@demobox
Copy link
Member

demobox commented May 13, 2014

jclouds » jclouds-labs #1029 FAILURE

Agree that this looks like a BuildHive problem again, @igreenfield. Let's wait to see what the DEV@cloud builder says...

@cloudbees-pull-request-builder

jclouds-labs-pull-requests #148 UNSTABLE
Looks like there's a problem with this pull request

@demobox
Copy link
Member

demobox commented May 14, 2014

jclouds-labs-pull-requests #148 UNSTABLE

This looks like a real test failure? There's also a bunch of Checkstyle violations.

Could you possibly have a look at those, @igreenfield? If there are any questions about the test failure or the Checkstyle complaints, of course please let us know!

@igreenfield
Copy link
Author

I removed the copyright.

@buildhive
Copy link

jclouds » jclouds-labs #1031 FAILURE
Looks like there's a problem with this pull request
(what's this?)

@cloudbees-pull-request-builder

jclouds-labs-pull-requests #149 FAILURE
Looks like there's a problem with this pull request

@igreenfield
Copy link
Author

@demobox After remove the copyright:
1 Unknown Licenses


Unapproved licenses:

/scratch/jenkins/workspace/jclouds/jclouds-labs/vsphere/pom.xml

@buildhive
Copy link

jclouds » jclouds-labs #1032 SUCCESS
This pull request looks good
(what's this?)

@cloudbees-pull-request-builder

jclouds-labs-pull-requests #150 SUCCESS
This pull request looks good

@buildhive
Copy link

jclouds » jclouds-labs #1033 SUCCESS
This pull request looks good
(what's this?)

@cloudbees-pull-request-builder

jclouds-labs-pull-requests #151 SUCCESS
This pull request looks good

@nacx
Copy link
Member

nacx commented Jun 5, 2014

Thanks for the contribution @igreenfield! Before going through the code to review it, there are a couple things to consider:

Licensing
According to the ASF Source Header and Copyright Notice Policy, this contribution does not fall into the "third party work" category, so all copyright notices must be removed from all header files. This is something that must be changed, in order to be aligned with the header policy.

Authorship attribution
As I've read in the previous PR in the main repository, this pull request is based on @andreaturli's work. We are a community of companies and individuals developing an OSS product, and proper attribution and credit to the original authors if one of the pillars of respect in this kind of communities. In fact it is one of the main points of every OSS license.

There have been several discussions in the ASF about the proper way to give credit to the submitted code. One of the points mentioned in that discussions was the convenience of including the @author tags in the Javadoc comments, and a widely accepted conclusion was to get rid of them. And we've recently started to apply this in the new pull requests.

To make sure your work here and Andrea's one is properly referenced, could you remove the @author tags from your classes, and add a mention to Andrea's work in the commit message so the code properly reflects the authors?

@igreenfield
Copy link
Author

Could you explain the Licensing section?

@nacx
Copy link
Member

nacx commented Jun 5, 2014

Sure.

This contribution you're submitting falls in the "Source File Headers for Code Developed at the ASF" category of the mentioned policy. Please, read the link in my comment. I assume you are the author of that code (regardless of if it is based on Andrea's one), so it is not a "third party submission". According to this, you should take care of addressing all points under the "Source File Headers for Code Developed at the ASF" category. Could you please do that?

@igreenfield
Copy link
Author

I am working on this code on my time at CISCO LTD. and I need to add this some where, where I can add this?

@nacx
Copy link
Member

nacx commented Jun 5, 2014

Contributions made to the ASF must be under the terms of the Contributor License Agreements. That agreement implies a copyright and patent grant to the Apache Software Foundation for all software you and your employer contributes to the ASF.

This means that the actual copyright owner of the software will be the ASF, and there's no need to include other copyright notices. NOTICE files are meant to reflect licensing and copyright details that must be kept for legal reasons (for example details from third party dependencies that are used by the project), but should not duplicate the terms of the LICENSE or include what it is already covered in the License Agreement.

In this case, the first steps to get everything in place so this contribution can be properly merged should be:

@nacx
Copy link
Member

nacx commented Jun 5, 2014

We've been discussing the licensing thing in the dev mailing list (please, read the entire thread) to clarify how your contribution should be properly licensed. There is no need to send a CCLA, because the contribution terms are already covered in section 5 of the license. The copyright note in the header files, though, must be removed to meet the header policy.

Regarding the "where you can add the copyright" question, and Quoting David:

It's generally frowned upon, because we build software as a community, but the contributor (or their company) still owns the copyright and can ask for that to be explicitly called out.

If you or your company want the attribution note to be explicitly present, you could add it to a NOTICE file in the root of the project. But as per David's comment, we'd encourage you and your employer not to do that and just remove it from the header file, as all other individuals and companies that have contributed to jclouds have done.

@demobox
Copy link
Member

demobox commented Jun 5, 2014

Thank you for your detailed research on the licensing topic, @nacx!

@cloudbees-pull-request-builder

jclouds-labs-pull-requests #290 FAILURE
Looks like there's a problem with this pull request

@buildhive
Copy link

jclouds » jclouds-labs #1609 SUCCESS
This pull request looks good
(what's this?)

@demobox
Copy link
Member

demobox commented Oct 12, 2014

Hi @igreenfield!

Thanks for sticking with this for so long! I'd briefly like to step back from the code for a second and get a better understanding of the background to the PR:

  • are you adding this based on user demand, or to provide a multi-cloud Java client for vSphere, or for some other reason?
  • do you know what plans there are to help ensure this provider stays up-to-date in future?
  • is there a chance of getting the jclouds team access to a testing instance and account so we can run live tests for this?

@if6was9
Copy link

if6was9 commented Oct 12, 2014

Getting a working working, supported vSphere support is very compelling for doing integration and automation in environments that have no IaaS support...which is most vCenter deployments.

The vSphere API is extremely complicated and unpleasant to use.

@igreenfield
Copy link
Author

Hi @demobox

@cloudbees-pull-request-builder

jclouds-labs-pull-requests #328 FAILURE
Looks like there's a problem with this pull request

@buildhive
Copy link

jclouds » jclouds-labs #1727 SUCCESS
This pull request looks good
(what's this?)

@igreenfield
Copy link
Author

@igreenfield
Copy link
Author

Hi @nacx ,
The merge turn to be very URGENT for me,
My dependent project need to be built and I need the snapshot version of this JAR.
thanks
Izek

* Add checking configuration of devices.
@cloudbees-pull-request-builder

jclouds-labs-pull-requests #332 FAILURE
Looks like there's a problem with this pull request

@buildhive
Copy link

jclouds » jclouds-labs #1742 SUCCESS
This pull request looks good
(what's this?)

@igreenfield
Copy link
Author

@demobox jdk1.7.0_72 solve the build problem

@nacx
Copy link
Member

nacx commented Oct 26, 2014

Hi @igreenfield @if6was9 ,

I understand that the merge can be urgent, but many of the comments have still not being addressed (for example, the interfaces coupled to vijava).

There have been recently several discussions about the providers in labs. You'll have seen that we've removed many of them, such as virtualbox, fcgp, jenkins, opsource-servers, and more. This is due to the fact that the labs repo has become a place where obsolete providers remain forever without maintenance, and make it really difficult to evolve the jclouds core and the main providers. Furthermore, several providers in labs are being used also in production, which makes it impossible to us to address major refactors without breaking many legacy (and unmaintained) code.

The jclouds-labs repository should be a temporal place where providers can be tested, but with a limited TTL to be promoted to the main repo, or removed. This will improve the overall quality of jclouds itself, and users will have a clear understanding on the state of each provider.

This said, I hope you understand that we can't merge providers in a state that is unlikely to be aligned with the way things are done in jclouds in a reasonable amount of time. I really appreciate all the effort that is being put in this PR, and I hope the reviews helped, but a project in this structure has no hope of being promoted out of labs. I suggest you host this code and publish your own snapshots. We will gladly send contributors your way if asked, but our only sustainable support for vsphere are cloud controllers such as cloudstack and openstack nova.

See also:
http://markmail.org/message/cdjk4qvvzbd5npye
http://markmail.org/message/hiqppxx3ie6e7bya

@igreenfield
Copy link
Author

OK,
I was better to said this at the start.
I will put this some where else.

From: Ignasi Barrera [mailto:notifications@github.com]
Sent: Sunday, October 26, 2014 11:56 AM
To: jclouds/jclouds-labs
Cc: Izek Greenfield (igreenfi)
Subject: Re: [jclouds-labs] Add vSphere support (#61)

Hi @igreenfieldhttps://github.com/igreenfield @if6was9https://github.com/if6was9 ,

I understand that the merge can be urgent, but many of the comments have still not being addressed (for example, the interfaces coupled to vijava).

There have been recently several discussions about the providers in labs. You'll have seen that we've removed many of them, such as virtualbox, fcgp, jenkins, opsource-servers, and more. This is due to the fact that the labs repo has become a place where obsolete providers remain forever without maintenance, and make it really difficult to evolve the jclouds core and the main providers. Furthermore, several providers in labs are being used also in production, which makes it impossible to us to address major refactors without breaking many legacy (and unmaintained) code.

The jclouds-labs repository should be a temporal place where providers can be tested, but with a limited TTL to be promoted to the main repo, or removed. This will improve the overall quality of jclouds itself, and users will have a clear understanding on the state of each provider.

This said, I hope you understand that we can't merge providers in a state that is unlikely to be aligned with the way things are done in jclouds in a reasonable amount of time. I really appreciate all the effort that is being put in this PR, and I hope the reviews helped, but a project in this structure has no hope of being promoted out of labs. I suggest you host this code and publish your own snapshots. We will gladly send contributors your way if asked, but our only sustainable support for vsphere are cloud controllers such as cloudstack and openstack nova.

See also:
http://markmail.org/message/cdjk4qvvzbd5npye
http://markmail.org/message/hiqppxx3ie6e7bya


Reply to this email directly or view it on GitHubhttps://github.com//pull/61#issuecomment-60512184.

@nacx
Copy link
Member

nacx commented Oct 26, 2014

My apologies for that.

The "recover the labs essence" discussion was started after this PR was initially reviewed, and although the initial expectations have changed I hope people will understand that the change is for the benefit of jclouds.

I really appreciate all the effort here and hope te review process has helped to put the vSphere provider in a better status.

* move to Guava 1.7
* Fix BUG in getting host parameters.
@cloudbees-pull-request-builder

jclouds-labs-pull-requests #355 FAILURE
Looks like there's a problem with this pull request

@buildhive
Copy link

@cloudbees-pull-request-builder

jclouds-labs-pull-requests #358 FAILURE
Looks like there's a problem with this pull request

@buildhive
Copy link

jclouds » jclouds-labs #1809 SUCCESS
This pull request looks good
(what's this?)

@cloudbees-pull-request-builder

jclouds-labs-pull-requests #360 FAILURE
Looks like there's a problem with this pull request

@buildhive
Copy link

jclouds » jclouds-labs #1813 SUCCESS
This pull request looks good
(what's this?)

@codefromthecrypt
Copy link
Contributor

Thanks @igreenfield and agreed that we should have especially been careful around the vijava thing as this was attempted and rejected in the past. You'll notice we've also just dropped virtualbox for similar reasons. I'll point anyone who asks about jclouds vsphere to you, and thanks for giving it a go!

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