You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by lahirus <no...@github.com> on 2014/03/27 19:25:22 UTC
[jclouds] Fixing Jclouds-509 (#333)
I have fixed imports movements.
You can merge this Pull Request by running:
git pull https://github.com/lahirus/jclouds master
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/333
-- Commit Summary --
* Fixing Jclouds-509
-- File Changes --
M providers/aws-ec2/pom.xml (2)
M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/AWSEC2ApiMetadata.java (2)
M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/binders/BindLaunchSpecificationToFormParams.java (13)
M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/AWSEC2TemplateOptions.java (59)
M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/strategy/AWSEC2CreateNodesInGroupThenAddToSet.java (1)
M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/strategy/CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions.java (16)
M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/domain/AWSRunningInstance.java (29)
M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/domain/LaunchSpecification.java (47)
M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/options/AWSRunInstancesOptions.java (29)
M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/xml/LaunchSpecificationHandler.java (10)
M providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/features/PlacementGroupApiExpectTest.java (4)
M providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/features/SpotInstanceApiExpectTest.java (4)
M providers/aws-ec2/src/test/resources/describe_instances_1.xml (2)
M providers/aws-ec2/src/test/resources/describe_instances_2.xml (2)
M providers/aws-ec2/src/test/resources/describe_instances_3.xml (2)
M providers/aws-ec2/src/test/resources/describe_instances_latest.xml (2)
M providers/aws-ec2/src/test/resources/describe_instances_pending.xml (2)
M providers/aws-ec2/src/test/resources/describe_security_groups_vpc.xml (2)
M providers/aws-ec2/src/test/resources/describe_spot_instance.xml (2)
M providers/aws-ec2/src/test/resources/describe_spot_instance_requests.xml (2)
M providers/aws-ec2/src/test/resources/describe_spot_instance_tags.xml (2)
M providers/aws-ec2/src/test/resources/describe_spot_instances_1.xml (2)
M providers/aws-ec2/src/test/resources/describe_spot_price_history.xml (2)
M providers/aws-ec2/src/test/resources/request_spot_instances-ebs.xml (2)
M providers/aws-ec2/src/test/resources/request_spot_instances.xml (2)
M providers/aws-ec2/src/test/resources/run_instances_1.xml (2)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/333.patch
https://github.com/jclouds/jclouds/pull/333.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Everett Toews <no...@github.com>.
It's release time in jclouds and that means we like to do a little house cleaning by sweeping out the pull request queue. This PR hasn't been touched in over 6 months. Please give us a status update.
If we don't hear anything by the end of the week, we'll close this pull request.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-59872288
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by lahirus <no...@github.com>.
Hi,
Sorry that i could not reply early. I tried building with clean repository. I could not reproduce any test failure :(.
Ah.. ok.. i will improve the commit message.
Thanks.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40252607
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1134](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1134/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-44049531
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1306](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1306/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-44318658
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Andrew Phillips <no...@github.com>.
> Will try to run a live test. It might take a bit time for me :( ..
Have you had any chance to look into this, @lahirus?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40230928
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #974](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/974/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-38848857
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1187](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1187/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-38850335
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1055](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1055/) FAILURE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-41781783
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #761](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/761/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40665662
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -182,7 +197,7 @@ public AWSEC2TemplateOptions subnetId(String subnetId) {
> /**
> * @see org.jclouds.aws.ec2.options.AWSRunInstancesOptions#withIAMInstanceProfileArn(String)
> */
> - @SinceApiVersion("2012-06-01")
> + @SinceApiVersion("2014-02-01")
This shouldn't be changed. This annotation is used to inform since which version of the provider API the method is available. Was it wrong?
If not, all `@SinceApiVersion` annotations should remain unchanged, even if you change the default api version to use in the `ApiMEtadata`.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333/files#r11671156
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Ignasi Barrera <no...@github.com>.
First of all thanks for the contribution @lahirus!
There were already many live tests failing in master, so I assume those failures aren't related to your changes. However, before merging this pull requests, some unit tests that assert your additions should be added to the following classes
* *BindLaunchSpecificationToFormParamsTest*
* *CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsTest* (simply add a method that tests the `addSecurityGroups` method)
* *AWSRunInstancesOptionsTest*
* *SpotInstanceApiExpectTest* There are no tests for the `requestSpotInstancesInRegion` (our fault), but having a method that tests it with a `LaunchSpecification` that uses your additions is important. Expect tests are end-to-end (mock-based) tests that verify the entire mechanism is working. If the test works, we know the REST API calls are being properly generated.
* *LaunchSpecificationHandlerTest* This test class does not exist, and I understand it is not in the scope of this PR to add all missing tests. However, if you could add a test there to verify the behavior of your concrete additions, that would be great.
We rely a lot on unit/live tests before releasing, specially when one supports such an amount of providers. It is important to add tests to every change so we don't break them unintentionally. Could you kindly add the relevant unit tests?
Thanks again @lahirus!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40567607
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #720](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/720/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-39053897
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -65,8 +65,17 @@
> if (launchSpec.getSecurityGroupIds().size() > 0)
> options.withSecurityGroupIds(launchSpec.getSecurityGroupIds());
> options.asType(checkNotNull(launchSpec.getInstanceType(), "instanceType"));
> - if (launchSpec.getSubnetId() != null)
> - options.withSubnetId(launchSpec.getSubnetId());
> + if (launchSpec.getSubnetId() != null){
> + if (Boolean.TRUE.equals(launchSpec.isPublicIpAddressAssociated())) {
> + options.associatePublicIpAddressAndSubnetId(launchSpec.getSubnetId());
> + if (launchSpec.getSecurityGroupIds().size() > 0){
> + options.withSecurityGroupIdsForNetworkInterface(launchSpec.getSecurityGroupIds());
> + }
> + }
> + else{
> + options.withSubnetId(launchSpec.getSubnetId());
> + }
> + }
[minor] Style: add a space before the `{` in the conditionals and format the code to use a 3 space indent.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333/files#r11671131
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Andrew Phillips <no...@github.com>.
> Here is the results from live test, There were 19 failures and one skipped.
Ah, great, thanks for that. @abayer @nacx Is this what we'd be expected to see here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40505544
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Lahiru Sandaruwan <no...@github.com>.
Okay.. i think this is expecting few more test cases to be completed. I
might not able to attend it this week. I'll try to re-send this with test
cases ASAP.
Thanks.
On Tue, Oct 21, 2014 at 8:29 AM, Everett Toews <no...@github.com>
wrote:
> It's release time in jclouds and that means we like to do a little house
> cleaning by sweeping out the pull request queue. This PR hasn't been
> touched in over 6 months. Please give us a status update.
>
> If we don't hear anything by the end of the week, we'll close this pull
> request.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/jclouds/jclouds/pull/333#issuecomment-59872288>.
>
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-60281038
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Andrew Phillips <no...@github.com>.
> jclouds-java-7-pull-requests #1292 SUCCESS
No test failures this time. Curious to see if this stays this way...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-44058807
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Andrew Phillips <no...@github.com>.
"Real" [compilation failures](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1055/console) here:
```
[INFO] 5 warnings
[INFO] -------------------------------------------------------------
[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR] /scratch/jenkins/workspace/jclouds/jclouds/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/strategy/CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsTest.java:[859,46] 'void' type not allowed here
[INFO] 1 error
[INFO] -------------------------------------------------------------
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-41866723
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1152](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1152/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-44315237
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Everett Toews <no...@github.com>.
Closed #333.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#event-191653695
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Andrew Phillips <no...@github.com>.
> jclouds-java-7-pull-requests #1306 UNSTABLE
The usual [Three Azure Failures](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1306/testReport/) of the Parallel Apocalypse...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-44322921
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #835](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/835/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-44317170
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Ignasi Barrera <no...@github.com>.
The PR changes more things apart from the binder, and I think there should be, at least, the following tests, especially the expect ones:
>* CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsTest (simply add a method that tests the addSecurityGroups method)
* AWSRunInstancesOptionsTest
* SpotInstanceApiExpectTest There are no tests for the requestSpotInstancesInRegion (our fault), but having a method that tests it with a LaunchSpecification that uses your additions is important. Expect tests are end-to-end (mock-based) tests that verify the entire mechanism is working. If the test works, we know the REST API calls are being properly generated.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-44162311
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by lahirus <no...@github.com>.
I was able to add test for "BindLaunchSpecificationToFormParamsTest" class. Do we have any test to test other usecases of "addSecurityGroups"? I'm trying to understand the model it works, since it is bit complex to me :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40843680
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Andrew Phillips <no...@github.com>.
> + // setup expectations
> + expect(options.getGroupIds()).andReturn(ImmutableSet.<String> of());
> + expect(options.getGroups()).andReturn(groupNames).atLeastOnce();
> + expect(options.getInboundPorts()).andReturn(ports).atLeastOnce();
> + RegionNameAndIngressRules regionNameAndIngressRules = new RegionNameAndIngressRules(region, generatedMarkerGroup,
> + ports, shouldAuthorizeSelf);
> + expect(strategy.securityGroupMap.getUnchecked(regionNameAndIngressRules)).andReturn(generatedMarkerGroup);
> +
> + // replay mocks
> + replay(options);
> + replayStrategy(strategy);
> +
> + // run
> +
> + RunInstancesOptions customize = strategy.execute(region, group, template);
> + assertEquals(strategy.addSecurityGroups(region, group, template, customize), returnVal);
Looks like this is causing the compilation failure? `addSecurityGroups` returns `void`.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333/files#r12172928
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1292](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1292/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-44051429
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1291](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1291/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-43978380
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by lahirus <no...@github.com>.
Hi,
Could not find exact instruction run live test from the link provided. Is it mentioned in Apache documentation site?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40425442
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #776](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/776/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-41780038
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Andrew Phillips <no...@github.com>.
Thanks for the detailed review, @nacx!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40593176
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Andrew Phillips <no...@github.com>.
Will try to have a look at this soon, but could you in the meantime clean up the four small [Checkstyle violations](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/717/org.apache.jclouds.provider$aws-ec2/violations/) that have been introduced?
Thanks, @lahirus!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-38882563
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Ignasi Barrera <no...@github.com>.
> return false;
> - } else if (!monitoringEnabled.equals(other.monitoringEnabled))
> - return false;
> - if (ramdiskId == null) {
> + if (publicIpAddressAssociated == null) {
> + if (other.publicIpAddressAssociated != null)
> + return false;
> + } else if (!publicIpAddressAssociated.equals(other.publicIpAddressAssociated))
> + return false;
Could you properly indent these two blocks? They are difficult to read.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333/files#r11671246
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1231](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1231/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40666087
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1190](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1190/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-39054073
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #717](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/717/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-38849202
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by lahirus <no...@github.com>.
> @@ -182,7 +197,7 @@ public AWSEC2TemplateOptions subnetId(String subnetId) {
> /**
> * @see org.jclouds.aws.ec2.options.AWSRunInstancesOptions#withIAMInstanceProfileArn(String)
> */
> - @SinceApiVersion("2012-06-01")
> + @SinceApiVersion("2014-02-01")
Oh. Got the idea. I will run a test with older version and commit.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333/files#r11671433
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by lahirus <no...@github.com>.
Thanks a lot for review @nacx. Will do these test case additions.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40617184
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1028](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1028/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40431491
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #822](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/822/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-44053001
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1226](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1226/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40432798
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -124,9 +124,13 @@ public void endElement(String uri, String name, String qName) {
> } else if (qName.equals("ramdiskId")) {
> builder.ramdiskId(currentOrNull());
> } else if (qName.equals("enabled")) {
> - String monitoringEnabled = currentOrNull();
> - if (monitoringEnabled != null)
> - builder.monitoringEnabled(Boolean.valueOf(monitoringEnabled));
> + String monitoringEnabled = currentOrNull();
> + if (monitoringEnabled != null) {
> + builder.monitoringEnabled(Boolean.valueOf(monitoringEnabled));}
> + }else if (qName.equals("publicIpAddressAssociated")) {
> + String publicIpAddressAssociated = currentOrNull();
> + if (publicIpAddressAssociated != null)
> + builder.publicIpAddressAssociated(Boolean.valueOf(publicIpAddressAssociated));
Same here: indent properly and add spaces in the conditionals.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333/files#r11671257
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Ignasi Barrera <no...@github.com>.
> - .subnetId("subnet-xyz").build();
> -
> - assertEquals(binder.apply(spec), ImmutableMap.of("LaunchSpecification.InstanceType", "t1.micro",
> - "LaunchSpecification.ImageId", "ami-123", "LaunchSpecification.SubnetId", "subnet-xyz"));
> - }
> + @Test
> + public void testApplyWithSubnetId() throws UnknownHostException {
> + LaunchSpecification spec = LaunchSpecification.builder().instanceType(InstanceType.T1_MICRO).imageId("ami-123")
> + .subnetId("subnet-xyz").build();
> +
> + assertEquals(binder.apply(spec), ImmutableMap.of("LaunchSpecification.InstanceType", "t1.micro",
> + "LaunchSpecification.ImageId", "ami-123", "LaunchSpecification.SubnetId", "subnet-xyz"));
> + }
> +
> + @Test
> + public void testApplyWithSubnetIdAnsPublicIpAssociation() throws UnknownHostException {
[minor] typo: "And"
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333/files#r13037796
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Andrew Phillips <no...@github.com>.
> + // setup expectations
> + expect(options.getGroupIds()).andReturn(ImmutableSet.<String> of());
> + expect(options.getGroups()).andReturn(groupNames).atLeastOnce();
> + expect(options.getInboundPorts()).andReturn(ports).atLeastOnce();
> + RegionNameAndIngressRules regionNameAndIngressRules = new RegionNameAndIngressRules(region, generatedMarkerGroup,
> + ports, shouldAuthorizeSelf);
> + expect(strategy.securityGroupMap.getUnchecked(regionNameAndIngressRules)).andReturn(generatedMarkerGroup);
> +
> + // replay mocks
> + replay(options);
> + replayStrategy(strategy);
> +
> + // run
> +
> + RunInstancesOptions customize = strategy.execute(region, group, template);
> + assertEquals(strategy.addSecurityGroups(region, group, template, customize), returnVal);
> I have mistakenly committed an half baked change.
No problem! Don't worry, there's no rush ;-) Thanks!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333/files#r12173707
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by lahirus <no...@github.com>.
Here is the results from live test, There were 19 failures and one skipped.
Failed tests: testAmazonLinuxInstanceStore(org.jclouds.aws.ec2.compute.AWSEC2TemplateBuilderLiveTest): expected [pv-2013.09.rc-1] but found [pv-2014.03.rc-1]
testFastestTemplateBuilder(org.jclouds.aws.ec2.compute.AWSEC2TemplateBuilderLiveTest): expected [cc2.8xlarge] but found [c3.8xlarge]
testTemplateBuilderWithLessRegions(org.jclouds.aws.ec2.compute.AWSEC2TemplateBuilderLiveTest): []
testTemplateBuilderCanFind(org.jclouds.aws.ec2.compute.AWSEC2TemplateBuilderLiveTest): no image matched predicate: And(nullEqualToIsParentOrIsGrandparentOfCurrentLocation(),And(osFamily(centos),osVersion(^6.0$),os64Bit(true)))
testTemplateBuilderCanFind(org.jclouds.aws.ec2.compute.AWSEC2TemplateBuilderLiveTest): no image matched predicate: And(nullEqualToIsParentOrIsGrandparentOfCurrentLocation(),And(osFamily(rhel),osVersion(^6.0$),os64Bit(true)))
testExtendedOptionsAndLogin(org.jclouds.aws.ec2.compute.AWSEC2ComputeServiceLiveTest): request POST https://ec2.us-west-2.amazonaws.com/ HTTP/1.1 failed with code 400, error: AWSError{requestId='8a543547-1fd9-47e3-89f4-c749ba13d683', requestToken='null', code='InvalidParameterValue', message='Value (0.3) for parameter price is invalid. Spot Instance request price "0.3" exceeds your maximum Spot price limit of "0.08"', context='{Response=, Errors=}'}
testTemplateBuilderCannotFind(org.jclouds.aws.ec2.compute.AWSEC2TemplateBuilderLiveTest): (..)
testTemplateBuilderCannotFind(org.jclouds.aws.ec2.compute.AWSEC2TemplateBuilderLiveTest): (..)
testTemplateBuilderCannotFind(org.jclouds.aws.ec2.compute.AWSEC2TemplateBuilderLiveTest): (..)
testTemplateBuilderCannotFind(org.jclouds.aws.ec2.compute.AWSEC2TemplateBuilderLiveTest): (..)
testTemplateBuilderCannotFind(org.jclouds.aws.ec2.compute.AWSEC2TemplateBuilderLiveTest): (..)
testTemplateBuilderCanUseImageIdWithoutFetchingAllImages(org.jclouds.aws.ec2.compute.AWSEC2TemplateBuilderLiveTest): []
testAutoIpAllocation(org.jclouds.aws.ec2.compute.AWSEC2ComputeServiceLiveTest): error running 1 node group(ec2-aip) location(us-east-1) image(ami-ad227cc4) size(t1.micro) options({blockOnPort:seconds=22:300, userDataCksum=2f4a740b})(..)
testAuthorizeSecurityGroupIngressCidr(org.jclouds.aws.ec2.features.AWSSecurityGroupApiLiveTest): request POST https://ec2.us-east-1.amazonaws.com/ HTTP/1.1 failed with code 503, error: AWSError{requestId='b74a4d59-de93-4e76-94d6-f478ebfbf8ef', requestToken='null', code='Unavailable', message='The service is unavailable. Please try again shortly.', context='{Response=, Errors=}'}
testCreateSpotInstance(org.jclouds.aws.ec2.features.SpotInstanceApiLiveTest): [region=sa-east-1, availabilityZoneGroup=lahiruec21, launchedAvailabilityZone=null, createTime=Mon Apr 14 16:55:22 MST 2014, faultCode=not-scheduled-yet, faultMessage=Your Spot request will not be evaluated until 2014-04-14T23:55:23+0000 due to your 'Valid From' constraint., instanceId=null, launchGroup=lahiruec21, launchSpecification=[instanceType=m1.small, imageId=ami-3e3be423, kernelId=null, ramdiskId=null, availabilityZone=sa-east-1b, subnetId=subnet-c32101aa, keyName=null, securityGroupIdToNames={sg-c2253cae=default}, blockDeviceMappings=[], securityGroupIds=[], securityGroupNames=[], monitoringEnabled=false, userData=null, iamInstanceProfile=null], productDescription=Linux/UNIX, id=sir-cd24f858, spotPrice=0.09, state=open, type=one-time, validFrom=null, validUntil=null, tags={}]
testDescribeSpotPriceHistoryInRegion(org.jclouds.aws.ec2.features.SpotInstanceApiLiveTest): [region=us-east-1, instanceType=c3.xlarge, productDescription=SUSE Linux, spotPrice=0.1328, timestamp=Mon Apr 14 16:54:39 MST 2014, availabilityZone=us-east-1b]
testDescribeSpotRequestsInRegionFilterInvalid(org.jclouds.aws.ec2.features.SpotInstanceApiLiveTest): (..)
testStartCCInstance(org.jclouds.aws.ec2.features.PlacementGroupApiLiveTest): org.jclouds.compute.RunNodesException: error running 1 node group(lahiruec2cccluster) location(us-east-1) image(ami-0da96764) size(cc2.8xlarge) options({scriptPresent=true, userDataCksum=2f4a740b})(..)
testListNodesByIds(org.jclouds.aws.ec2.compute.AWSEC2ComputeServiceLiveTest): nodes and listNodesByIds should be identical: was [] but should be [{id=us-east-1/i-42e7ed13, providerId=i-42e7ed13, name=first-node, location={scope=ZONE, id=us-east-1b, description=us-east-1b, parent=us-east-1, iso3166Codes=[US-VA]}, group=ec2-, imageId=us-east-1/ami-ad227cc4, os={family=amzn-linux, arch=paravirtual, version=vpc-nat-pv-2013.09.0, description=amazon/amzn-ami-vpc-nat-pv-2013.09.0.x86_64-ebs, is64Bit=true}, status=RUNNING[running], loginPort=22, hostname=domU-12-31-39-05-35-E3, privateAddresses=[10.241.54.17], publicAddresses=[54.224.11.213], hardware={id=t1.micro, providerId=t1.micro, processors=[{cores=1.0, speed=1.0}], ram=630, volumes=[{id=vol-f4838082, type=SAN, device=/dev/sda1, bootDevice=true, durable=true}], hypervisor=xen, supportsImage=And(requiresRootDeviceType(ebs),Or(isWindows(),requiresVirtualizationType(paravirtual)),ALWAYS_TRUE,ALWAYS_TRUE)}, loginUser=ec2-user, userMetada
ta={Name=first-node}}, {id=us-east-1/i-43e7ed12, providerId=i-43e7ed12, name=ec2--43e7ed12, location={scope=ZONE, id=us-east-1b, description=us-east-1b, parent=us-east-1, iso3166Codes=[US-VA]}, group=ec2-, imageId=us-east-1/ami-ad227cc4, os={family=amzn-linux, arch=paravirtual, version=vpc-nat-pv-2013.09.0, description=amazon/amzn-ami-vpc-nat-pv-2013.09.0.x86_64-ebs, is64Bit=true}, status=RUNNING[running], loginPort=22, hostname=domU-12-31-39-15-0E-3A, privateAddresses=[10.207.13.196], publicAddresses=[54.242.66.49], hardware={id=t1.micro, providerId=t1.micro, processors=[{cores=1.0, speed=1.0}], ram=630, volumes=[{id=vol-fb83808d, type=SAN, device=/dev/sda1, bootDevice=true, durable=true}], hypervisor=xen, supportsImage=And(requiresRootDeviceType(ebs),Or(isWindows(),requiresVirtualizationType(paravirtual)),ALWAYS_TRUE,ALWAYS_TRUE)}, loginUser=ec2-user, userMetadata={Name=ec2--43e7ed12}}, {id=us-east-1/i-44efe515, providerId=i-44efe515, name=ec2--44efe515, location={scope=ZONE, id=u
s-east-1b, description=us-east-1b, parent=us-east-1, iso3166Codes=[US-VA]}, group=ec2-, imageId=us-east-1/ami-ad227cc4, os={family=amzn-linux, arch=paravirtual, version=vpc-nat-pv-2013.09.0, description=amazon/amzn-ami-vpc-nat-pv-2013.09.0.x86_64-ebs, is64Bit=true}, status=RUNNING[running], loginPort=22, hostname=domU-12-31-39-04-6C-4E, privateAddresses=[10.240.115.188], publicAddresses=[54.204.160.1], hardware={id=t1.micro, providerId=t1.micro, processors=[{cores=1.0, speed=1.0}], ram=630, volumes=[{id=vol-b4fffcc2, type=SAN, device=/dev/sda1, bootDevice=true, durable=true}], hypervisor=xen, supportsImage=And(requiresRootDeviceType(ebs),Or(isWindows(),requiresVirtualizationType(paravirtual)),ALWAYS_TRUE,ALWAYS_TRUE)}, loginUser=lahiru, userMetadata={Name=ec2--44efe515}}, {id=us-east-1/i-4befe51a, providerId=i-4befe51a, name=ec2--4befe51a, location={scope=ZONE, id=us-east-1b, description=us-east-1b, parent=us-east-1, iso3166Codes=[US-VA]}, group=ec2-, imageId=us-east-1/ami-ad227cc4,
os={family=amzn-linux, arch=paravirtual, version=vpc-nat-pv-2013.09.0, description=amazon/amzn-ami-vpc-nat-pv-2013.09.0.x86_64-ebs, is64Bit=true}, status=RUNNING[running], loginPort=22, hostname=domU-12-31-39-06-C0-4C, privateAddresses=[10.208.199.186], publicAddresses=[54.198.105.133], hardware={id=t1.micro, providerId=t1.micro, processors=[{cores=1.0, speed=1.0}], ram=630, volumes=[{id=vol-6cfefd1a, type=SAN, device=/dev/sda1, bootDevice=true, durable=true}], hypervisor=xen, supportsImage=And(requiresRootDeviceType(ebs),Or(isWindows(),requiresVirtualizationType(paravirtual)),ALWAYS_TRUE,ALWAYS_TRUE)}, loginUser=lahiru, userMetadata={Name=ec2--4befe51a}}, {id=us-east-1/i-b0fcf6e1, providerId=i-b0fcf6e1, name=ec2--b0fcf6e1, location={scope=ZONE, id=us-east-1b, description=us-east-1b, parent=us-east-1, iso3166Codes=[US-VA]}, group=ec2-, imageId=us-east-1/ami-ad227cc4, os={family=amzn-linux, arch=paravirtual, version=vpc-nat-pv-2013.09.0, description=amazon/amzn-ami-vpc-nat-pv-2013.0
9.0.x86_64-ebs, is64Bit=true}, status=RUNNING[running], loginPort=22, hostname=domU-12-31-39-0A-54-C9, privateAddresses=[10.211.87.55], publicAddresses=[107.20.60.89], hardware={id=t1.micro, providerId=t1.micro, processors=[{cores=1.0, speed=1.0}], ram=630, volumes=[{id=vol-d78182a1, type=SAN, device=/dev/sda1, bootDevice=true, durable=true}], hypervisor=xen, supportsImage=And(requiresRootDeviceType(ebs),Or(isWindows(),requiresVirtualizationType(paravirtual)),ALWAYS_TRUE,ALWAYS_TRUE)}, loginUser=lahiru, userMetadata={Name=ec2--b0fcf6e1}}] expected [true] but found [false]
Tests run: 194, Failures: 19, Errors: 0, Skipped: 1
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40442733
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Everett Toews <no...@github.com>.
@lahirus Thanks for the note. I'll hold off until the end of next week then (Oct. 31).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-60289412
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #756](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/756/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40432444
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Ignasi Barrera <no...@github.com>.
I think we don't have other tests right now. Perhaps @abayer can help providing info about the different use cases.
Anyway, I'd start adding some tests that directly call the method with different inputs that make sense to your changes and verifying that the options are properly populated. Covering your changes is enough for this PR.
Thanks for taking care of this!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40844844
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -125,8 +125,12 @@ public void endElement(String uri, String name, String qName) {
> builder.ramdiskId(currentOrNull());
> } else if (qName.equals("enabled")) {
> String monitoringEnabled = currentOrNull();
> - if (monitoringEnabled != null)
> - builder.monitoringEnabled(Boolean.valueOf(monitoringEnabled));
> + if (monitoringEnabled != null) {
> + builder.monitoringEnabled(Boolean.valueOf(monitoringEnabled));}
> + }else if (qName.equals("publicIpAddressAssociated")) {
[minor] space after `{`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333/files#r13037802
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by lahirus <no...@github.com>.
@nacx I thought I can avoid "addSecurityGroups" test case as per one of your previous replies :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-45178771
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1133](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1133/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-43976114
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by lahirus <no...@github.com>.
Closed #333.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by lahirus <no...@github.com>.
Will do the style adjustments.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40567156
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Andrew Phillips <no...@github.com>.
> I can do another if you mean that.
If that is all that has changed, I don't think we need another test run.
@nacx Are your test cases covered?
@abayer Any thoughts on other steps to take to validate this PR, if any?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-44152808
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by lahirus <no...@github.com>.
Do you mean after this test case addition? I already did a one. I can do another if you mean that.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-44070662
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1246](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1246/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-41782708
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by lahirus <no...@github.com>.
Hi @demobox
I have fixed the Checkstyle violations.
Will try to run a live test. It might take a bit time for me :( ..
Thanks.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-39052252
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Andrew Gaul <no...@github.com>.
Can you also improve the commit message?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40252083
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Andrew Phillips <no...@github.com>.
> Is it mentioned in Apache documentation site?
You've caught us in a bit of an in-between state here. The [testing guide](https://github.com/jclouds/jclouds-site/blob/4bb3b95c39152828ffe6ae44d52b83f827f9fb8d/documentation/devguides/provider-testing.markdown#running-live-tests) is still being ported - that's a link to an older version, but the instructions there should still be accurate.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40425837
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1034](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1034/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40665021
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Andrew Phillips <no...@github.com>.
> jclouds » jclouds #978 UNSTABLE
> jclouds-java-7-pull-requests #1190 UNSTABLE
Both unrelated [test](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/org.apache.jclouds$jclouds-compute/1190/testReport/junit/org.jclouds.compute.util/ConcurrentOpenSocketFinderTest/testChecksSocketsConcurrently/) [failures](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/org.apache.jclouds$jclouds-compute/978/testReport/junit/org.jclouds.compute.callables/BlockUntilInitScriptStatusIsZeroThenReturnOutputTest/testloopUntilTrueOrThrowCancellationExceptionReturnsWhenPredicateIsTrueSecondTimeWhileNotCancelled/)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-39274804
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Andrew Phillips <no...@github.com>.
Another thing that we will certainly want to do, by the way, is run the live test suite against AWS EC2. See the ["Running live tests" section](http://2bfcdbe0cc28b674f568-6e84ef02f7d038e5f4526bc5b8a12a51.r1.cf1.rackcdn.com/documentation/devguides/provider-testing/) from this cached version of the old site.
If you could run those tests and paste the results here (there will be [some failures](https://issues.apache.org/jira/browse/JCLOUDS-462), by the way), that would be great. Otherwise, please let us know and we'll see how best to validate this against AWS.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-38882802
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Andrew Phillips <no...@github.com>.
Glad that we're back to a successfully building PR here, @lahirus! Do you have any live test output for this?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-44058892
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by lahirus <no...@github.com>.
> + // setup expectations
> + expect(options.getGroupIds()).andReturn(ImmutableSet.<String> of());
> + expect(options.getGroups()).andReturn(groupNames).atLeastOnce();
> + expect(options.getInboundPorts()).andReturn(ports).atLeastOnce();
> + RegionNameAndIngressRules regionNameAndIngressRules = new RegionNameAndIngressRules(region, generatedMarkerGroup,
> + ports, shouldAuthorizeSelf);
> + expect(strategy.securityGroupMap.getUnchecked(regionNameAndIngressRules)).andReturn(generatedMarkerGroup);
> +
> + // replay mocks
> + replay(options);
> + replayStrategy(strategy);
> +
> + // run
> +
> + RunInstancesOptions customize = strategy.execute(region, group, template);
> + assertEquals(strategy.addSecurityGroups(region, group, template, customize), returnVal);
Hi Andrew,
I have mistakenly committed an half baked change. Will fix asap.
Thanks.
Sent from my mobile.
On May 1, 2014 5:42 AM, "Andrew Phillips" <no...@github.com> wrote:
> In
> providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/strategy/CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsTest.java:
>
> > + // setup expectations
> > + expect(options.getGroupIds()).andReturn(ImmutableSet.<String> of());
> > + expect(options.getGroups()).andReturn(groupNames).atLeastOnce();
> > + expect(options.getInboundPorts()).andReturn(ports).atLeastOnce();
> > + RegionNameAndIngressRules regionNameAndIngressRules = new RegionNameAndIngressRules(region, generatedMarkerGroup,
> > + ports, shouldAuthorizeSelf);
> > + expect(strategy.securityGroupMap.getUnchecked(regionNameAndIngressRules)).andReturn(generatedMarkerGroup);
> > +
> > + // replay mocks
> > + replay(options);
> > + replayStrategy(strategy);
> > +
> > + // run
> > +
> > + RunInstancesOptions customize = strategy.execute(region, group, template);
> > + assertEquals(strategy.addSecurityGroups(region, group, template, customize), returnVal);
>
> Looks like this is causing the compilation failure? addSecurityGroupsreturns
> void.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/jclouds/jclouds/pull/333/files#r12172928>
> .
>
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333/files#r12173604
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Andrew Phillips <no...@github.com>.
Closed #333.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#event-124350951
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #821](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/821/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-43978362
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Ignasi Barrera <no...@github.com>.
I think I didn't explain properly what I wanted to say. We need the three tests I mentioned in my last comment.
Regarding the `addSecurityGroups` one, there are not tests for other use cases of that method, but we need at least the ones that validate the changes in this PR. What I suggested was to add a method to the test class that directly calls the `addSecurityGroups` method with different inputs that make sense to the `associatePublicIpAddress` use case and check that the results are the expected ones. There is no need to cover all the paths of that method in this PR. Just the ones related to your additions
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-45194369
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by lahirus <no...@github.com>.
Hi,
Thanks for looking into it. Will work on these.
Thanks.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-38884799
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #978](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/978/) UNSTABLE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-39053643
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by lahirus <no...@github.com>.
:) I was wondering if you have a sample for AWS there.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40442470
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Ignasi Barrera <no...@github.com>.
> if (subnetId != null) {
> - AWSRunInstancesOptions.class.cast(instanceOptions).withSubnetId(subnetId);
> + if(associatePublicIpAddress){
> + AWSRunInstancesOptions.class.cast(instanceOptions).associatePublicIpAddressAndSubnetId(subnetId);
> + if (awsTemplateOptions.getGroupIds().size() > 0)
> + awsInstanceOptions.withSecurityGroupIdsForNetworkInterface(awsTemplateOptions.getGroupIds());
> + }else{
> + AWSRunInstancesOptions.class.cast(instanceOptions).withSubnetId(subnetId);
> + if (awsTemplateOptions.getGroupIds().size() > 0)
> + awsInstanceOptions.withSecurityGroupIds(awsTemplateOptions.getGroupIds());
> + }
[minor] Style: Add the spaces in the conditionals.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333/files#r11671191
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by Andrew Phillips <no...@github.com>.
> jclouds-pull-requests #821 FAILURE
Looks like a [slave timeout](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/821/console) here - unrelated to the PR.
> jclouds-java-7-pull-requests #1291 FAILURE
These [test failures](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/org.apache.jclouds.common$azure-common/1291/testReport/org.jclouds.azure.storage.filters/SharedKeyLiteAuthenticationTest/testIdempotent/) are probably being caused by our parallel build experiment. I don't think they're related to this PR, either.
Let's close'n'reopen to see what happens.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-44043794
Re: [jclouds] Fixing Jclouds-509 (#333)
Posted by lahirus <no...@github.com>.
Reopened #333.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333