You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Andrew Bayer <no...@github.com> on 2013/11/01 01:13:00 UTC
[jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Note - this is just the first part mentioned in the JIRA - adding the filter support in the first place. I need to figure out how to do the rest - translating NodeMetadata Predicates into EC2 instance filters, which...well, it'll be a bit of a pain, but oh my will it be useful.
You can merge this Pull Request by running:
git pull https://github.com/abayer/jclouds-1 jclouds-361
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/194
-- Commit Summary --
* JCLOUDS-361. Add support for filter parameters to DescribeInstances for aws-ec2
-- File Changes --
M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/features/AWSInstanceApi.java (38)
A providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/features/AWSInstanceApiExpectTest.java (88)
M providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/features/AWSInstanceApiLiveTest.java (28)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/194.patch
https://github.com/jclouds/jclouds/pull/194.diff
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> @@ -58,4 +62,38 @@ void testDescribeInstances() {
> assert allResults.size() >= 0 : allResults.size();
> }
> }
> -}
> +
> + @Test
> + void testFilterInstances() {
> + for (String region : view.unwrapApi(EC2Api.class).getAvailabilityZoneAndRegionApi().get().describeRegions().keySet()) {
> + Set<? extends Reservation<? extends RunningInstance>> allResults = client.describeInstancesInRegion(region);
> + assertNotNull(allResults);
> + assert allResults.size() >= 0 : allResults.size();
Fixing.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7508557
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #374](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/374/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-28004260
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> + * @see #describeSnapshotsInRegion
> + * @see #createSnapshotsInRegion
> + * @see #deleteSnapshotInRegion
> + * @see <a href="http://docs.amazonwebservices.com/AWSEC2/latest/APIReference/ApiReference-query-DescribeSnapshots.html"
> + * />
> + */
> + @Named("DescribeSnapshots")
> + @POST
> + @Path("/")
> + @FormParams(keys = ACTION, values = "DescribeSnapshots")
> + @XMLResponseParser(DescribeSnapshotsResponseHandler.class)
> + @Fallback(EmptySetOnNotFoundOr404.class)
> + Set<Snapshot> describeSnapshotsInRegionWithFilter(
> + @EndpointParam(parser = RegionToEndpointOrProviderIfNull.class) @Nullable String region,
> + @BinderParam(BindFiltersToIndexedFormParams.class) Multimap<String, String> filter,
> + DescribeSnapshotsOptions... options);
Intentionally a varargs of DescribeSnapshotsOptions here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7486349
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> @@ -59,4 +62,27 @@ void testDescribeInstances() {
> }
> }
>
> + @Test
> + void testFilterInstances() {
> + for (String region : view.unwrapApi(AWSEC2Api.class).getAvailabilityZoneAndRegionApi().get().describeRegions().keySet()) {
> + Set<? extends Reservation<? extends RunningInstance>> allResults = client.describeInstancesInRegion(region);
> + assertNotNull(allResults);
> + assert allResults.size() >= 0 : allResults.size();
Use `assertTrue`? And `!allResults.isEmpty()`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7360935
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #829](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/829/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-27706736
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
Some repeated comments:
* use TestNG assertions only, at least in new test code (rather than Java `assert`s)?
* plenty of cases where assertions in a test are surrounded by `if (allResults.size() >= 1) { ... }`. Are we legitimately expecting no responses here? If so, at least throw a SkipException, rather than simply having a test succeed, even though no assertions were actually run?
* some test classes seem to make the tests package-level `void`? Use `public void` as in most other tests?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-27935365
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> + Set<? extends Image> filterResult = client.describeImagesInRegionWithFilter(region,
> + ImmutableMultimap.<String, String>builder()
> + .put("image-id", id1)
> + .build());
> + assertNotNull(filterResult);
> + assertEquals(filterResult.size(), 1);
> + iterator = filterResult.iterator();
> + assertEquals(iterator.next().getId(), id1);
> + }
> + }
> +
> + @Test(expectedExceptions = AWSResponseException.class)
> + public void testDescribeImagesWithInvalidFilter() {
> + // Just run in the first region - no need to take the time on all of them.
> + String region = getFirst(ec2Api.getConfiguredRegions(), null);
> + if (region != null) {
Yeah, that makes sense. I'm gonna change them all to barf if the region is null.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7508385
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> Rebased on master, pushing to master - we can nitpick it later if need be. =)
Thanks, @abayer! I think we should be OK ;-)
> I'd like to keep the varargs stuff, if that's alright, just so as not to break existing logic and the like.
Fair enough.
> The tests that loop and only assert if they find regions and then fail...I think we've got enough other stuff that's gonna barf if we don't find any regions that I'm ok with leaving them as is.
If we're reasonably confident that other stuff will then break...fine!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-28818392
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #366](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/366/) 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/194#issuecomment-27706687
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> .addFormParam("SignatureMethod", "HmacSHA256")
> .addFormParam("SignatureVersion", "2")
> .addFormParam("Timestamp", "2009-11-08T15%3A54%3A08.897Z")
> .addFormParam("UserGroup.1", "all")
> .addFormParam("UserId.1", "bob")
> .addFormParam("UserId.2", "sue")
> - .addFormParam("Version", "2010-06-15")
> + .addFormParam("Version", "2010-08-31")
Seems like this could be usefully factored out into a constant somewhere, at least for the tests..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7486426
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> + *
> + * @param region
> + * Snapshots are tied to Regions and can only be used for volumes within the same
> + * Region.
> + * @param filter
> + * Multimap of filter key/values.
> + * @param options
> + * specify the snapshot ids or other parameters to clarify the list.
> + * @return matching snapshots.
> + *
> + * @see #describeSnapshotsInRegion
> + * @see #createSnapshotsInRegion
> + * @see #deleteSnapshotInRegion
> + * @see <a href="http://docs.amazonwebservices.com/AWSEC2/latest/APIReference/ApiReference-query-DescribeSnapshots.html"
> + * />
> + */
Fixed.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7505121
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> +import org.testng.Assert;
> +import org.testng.annotations.Test;
> +
> +import com.google.common.collect.ImmutableMultimap;
> +import com.google.common.collect.ImmutableSet;
> +
> +/**
> + * @author Andrew Bayer
> + */
> +@Test(groups = "unit")
> +public class InstanceApiExpectTest extends BaseEC2ApiExpectTest<EC2Api> {
> + /**
> + * @see InstanceApi
> + * @see org.jclouds.rest.annotations.SinceApiVersion
> + */
> + protected Properties setupProperties() {
If anything, does this Javadoc belong on the _class_, rather than this method? Otherwise, remove?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7486483
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> @@ -55,7 +59,41 @@ void testDescribeInstances() {
> for (String region : ec2Api.getConfiguredRegions()) {
> Set<? extends Reservation<? extends RunningInstance>> allResults = client.describeInstancesInRegion(region);
> assertNotNull(allResults);
> - assert allResults.size() >= 0 : allResults.size();
> + assertTrue(allResults.size() >= 0);
How can a set be of a negative size?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7513180
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> + .build());
> + assertNotNull(filterResult);
> + assertEquals(filterResult.size(), 1);
> + iterator = filterResult.iterator();
> + assertEquals(iterator.next().getId(), id1);
> + }
> + }
> +
> + @Test(expectedExceptions = AWSResponseException.class)
> + public void testDescribeImagesWithInvalidFilter() {
> + // Just run in the first region - no need to take the time on all of them.
> + String region = getFirst(ec2Api.getConfiguredRegions(), null);
> + if (region != null) {
> + Set<? extends Image> allResults = client.describeImagesInRegion(region);
> + assertNotNull(allResults);
> + assert allResults.size() >= 2 : allResults.size();
Use TestNG Asserts?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7486418
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by BuildHive <no...@github.com>.
[jclouds ยป jclouds #618](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/618/) 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/194#issuecomment-28513772
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> @@ -224,15 +226,23 @@ void deleteSecurityGroup(String region, String group) {
>
> @VisibleForTesting
> void deleteKeyPair(String region, String group) {
> - for (KeyPair keyPair : client.getKeyPairApi().get().describeKeyPairsInRegion(region)) {
> + for (KeyPair keyPair : client.getKeyPairApi().get().describeKeyPairsInRegionWithFilter(region,
> + ImmutableMultimap.<String, String>builder()
> + .put("key-name", Strings2.urlEncode(
> + String.format("jclouds#%s#%s*", group, region).replace('#', delimiter)))
Are there more `#` characters expected in `"jclouds#%s#%s*"`? Otherwise, why not insert the delimiter straight away?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7486282
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> @@ -224,15 +226,23 @@ void deleteSecurityGroup(String region, String group) {
>
> @VisibleForTesting
> void deleteKeyPair(String region, String group) {
> - for (KeyPair keyPair : client.getKeyPairApi().get().describeKeyPairsInRegion(region)) {
> + for (KeyPair keyPair : client.getKeyPairApi().get().describeKeyPairsInRegionWithFilter(region,
> + ImmutableMultimap.<String, String>builder()
> + .put("key-name", Strings2.urlEncode(
> + String.format("jclouds#%s#%s*", group, region).replace('#', delimiter)))
OK, then let's leave as-is.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7513025
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #359](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/359/) 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/194#issuecomment-27540442
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> @@ -75,16 +77,44 @@ public void setupContext() {
>
> @Test
> void testDescribeVolumes() {
> - for (String region : ec2Api.getConfiguredRegions()) {
> - SortedSet<Volume> allResults = Sets.newTreeSet(client.describeVolumesInRegion(region));
> - assertNotNull(allResults);
> - if (allResults.size() >= 1) {
> - Volume volume = allResults.last();
> - SortedSet<Volume> result = Sets.newTreeSet(client.describeVolumesInRegion(region, volume.getId()));
> - assertNotNull(result);
> - Volume compare = result.last();
> - assertEquals(compare, volume);
> - }
> + String region = defaultRegion;
> + SortedSet<Volume> allResults = Sets.newTreeSet(client.describeVolumesInRegion(region));
> + assertNotNull(allResults);
> + if (allResults.size() >= 1) {
Yup, fixing that logic.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7508511
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
Rebased on master, pushing to master - we can nitpick it later if need be. =)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-28516774
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> @@ -58,4 +62,38 @@ void testDescribeInstances() {
> assert allResults.size() >= 0 : allResults.size();
> }
> }
> -}
> +
> + @Test
> + void testFilterInstances() {
> + for (String region : view.unwrapApi(EC2Api.class).getAvailabilityZoneAndRegionApi().get().describeRegions().keySet()) {
> + Set<? extends Reservation<? extends RunningInstance>> allResults = client.describeInstancesInRegion(region);
> + assertNotNull(allResults);
> + assert allResults.size() >= 0 : allResults.size();
> +
> + if (!allResults.isEmpty()) {
Again: do we ever legitimately expect no results here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7486492
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #828](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/828/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-27660123
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> }
> }
> -}
> +
> + @Test
> + void testFilterInstances() {
> + for (String region : view.unwrapApi(EC2Api.class).getAvailabilityZoneAndRegionApi().get().describeRegions().keySet()) {
> + Set<? extends Reservation<? extends RunningInstance>> allResults = client.describeInstancesInRegion(region);
> + assertNotNull(allResults);
> + assertTrue(allResults.size() >= 0);
How can a set be of a negative size?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7513186
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> @@ -132,11 +138,55 @@ public void testDescribeImages() {
> }
>
> @Test
> + public void testDescribeImagesWithFilter() {
> + // Just run in the first region - no need to take the time on all of them.
> + String region = getFirst(ec2Api.getConfiguredRegions(), null);
> + if (region != null) {
> + Set<? extends Image> allResults = client.describeImagesInRegion(region);
> + assertNotNull(allResults);
> + assert allResults.size() >= 2 : allResults.size();
Yeah, I inherited that stuff - fixing.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7505190
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> @@ -115,7 +119,9 @@ public void testDescribeImageBadId() {
> }
>
> public void testDescribeImages() {
> - for (String region : ec2Api.getConfiguredRegions()) {
> + // Just run in the first region - no need to take the time on all of them.
> + String region = getFirst(ec2Api.getConfiguredRegions(), null);
> + if (region != null) {
Functional change in the test here...are we sure the result will be the same across regions?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7486381
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #830](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/830/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-27727447
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> @@ -132,11 +138,55 @@ public void testDescribeImages() {
> }
>
> @Test
> + public void testDescribeImagesWithFilter() {
> + // Just run in the first region - no need to take the time on all of them.
> + String region = getFirst(ec2Api.getConfiguredRegions(), null);
> + if (region != null) {
> + Set<? extends Image> allResults = client.describeImagesInRegion(region);
> + assertNotNull(allResults);
> + assert allResults.size() >= 2 : allResults.size();
Use TestNG Asserts?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7486399
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #827](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/827/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-27659544
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> @@ -66,6 +69,39 @@
> @EndpointParam(parser = RegionToEndpointOrProviderIfNull.class) @Nullable String region,
> @BinderParam(BindInstanceIdsToIndexedFormParams.class) String... instanceIds);
>
> + /**
> + * Returns information about instances that you own.
> + * <p/>
> + *
> + * If you specify one or filters, Amazon EC2 returns information for instances
> + * matching those filters. If you do not specify any filters, Amazon EC2
> + * returns information for all relevant instances. If you specify an invalid
> + * filter, a fault is returned. Only instances you own will be included in the
I'm not entirely sure. That's tweaked copytext from the describeInstancesInRegion method - I'll play with it and figure out exactly what happens.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7360913
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> + *
> + * @param region
> + * Snapshots are tied to Regions and can only be used for volumes within the same
> + * Region.
> + * @param filter
> + * Multimap of filter key/values.
> + * @param options
> + * specify the snapshot ids or other parameters to clarify the list.
> + * @return matching snapshots.
> + *
> + * @see #describeSnapshotsInRegion
> + * @see #createSnapshotsInRegion
> + * @see #deleteSnapshotInRegion
> + * @see <a href="http://docs.amazonwebservices.com/AWSEC2/latest/APIReference/ApiReference-query-DescribeSnapshots.html"
> + * />
> + */
Formatting
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7486344
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #361](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/361/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-27610918
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> }
> }
> -}
> +
> + @Test
> + void testFilterInstances() {
> + for (String region : view.unwrapApi(EC2Api.class).getAvailabilityZoneAndRegionApi().get().describeRegions().keySet()) {
> + Set<? extends Reservation<? extends RunningInstance>> allResults = client.describeInstancesInRegion(region);
> + assertNotNull(allResults);
> + assertTrue(allResults.size() >= 0);
I...have no idea. Ha.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7667518
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #364](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/364/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-27659489
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> @@ -132,11 +138,55 @@ public void testDescribeImages() {
> }
>
> @Test
> + public void testDescribeImagesWithFilter() {
> + // Just run in the first region - no need to take the time on all of them.
> + String region = getFirst(ec2Api.getConfiguredRegions(), null);
> + if (region != null) {
Lemme think about that a bit. I think I just have the if statement in there for paranoia's sake, but I think you're right.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7505181
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #837](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/837/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-28004477
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
Ok, this is the for real one now. =)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-27706228
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> @@ -132,11 +138,55 @@ public void testDescribeImages() {
> }
>
> @Test
> + public void testDescribeImagesWithFilter() {
> + // Just run in the first region - no need to take the time on all of them.
> + String region = getFirst(ec2Api.getConfiguredRegions(), null);
> + if (region != null) {
> + Set<? extends Image> allResults = client.describeImagesInRegion(region);
> + assertNotNull(allResults);
> + assert allResults.size() >= 2 : allResults.size();
> + Iterator<? extends Image> iterator = allResults.iterator();
okiedokie.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7508374
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> @@ -110,6 +112,41 @@ void testDescribe() {
> }
>
> @Test
> + void testFilter() {
Done.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7508665
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #825](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/825/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-27656434
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> @@ -66,6 +69,39 @@
> @EndpointParam(parser = RegionToEndpointOrProviderIfNull.class) @Nullable String region,
> @BinderParam(BindInstanceIdsToIndexedFormParams.class) String... instanceIds);
>
> + /**
> + * Returns information about instances that you own.
> + * <p/>
> + *
> + * If you specify one or filters, Amazon EC2 returns information for instances
> + * matching those filters. If you do not specify any filters, Amazon EC2
> + * returns information for all relevant instances. If you specify an invalid
> + * filter, a fault is returned. Only instances you own will be included in the
> + * results.
> + * <p/>
> + * Recently terminated instances might appear in the returned results.This
Fixed.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7360921
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
Ah, found a couple more of the size guys - dealt with 'em in next commit.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-28508179
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #363](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/363/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-27656693
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #399](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/399/) 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/194#issuecomment-28518357
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> + * @see InstanceApi#describeInstances
> + * @see #describeImageAttribute
> + * @see <a href="http://docs.amazonwebservices.com/AWSEC2/latest/APIReference/ApiReference-query-DescribeImages.html"
> + * />
> + * @see DescribeImagesOptions
> + */
> + @Named("DescribeImages")
> + @POST
> + @Path("/")
> + @FormParams(keys = ACTION, values = "DescribeImages")
> + @XMLResponseParser(DescribeImagesResponseHandler.class)
> + @Fallback(EmptySetOnNotFoundOr404.class)
> + Set<? extends Image> describeImagesInRegionWithFilter(
> + @EndpointParam(parser = RegionToEndpointOrProviderIfNull.class) @Nullable String region,
> + @BinderParam(BindFiltersToIndexedFormParams.class) Multimap<String, String> filter,
> + DescribeImagesOptions... options);
If you look at describeImagesInRegion, there's a varargs array of DescribeImagesOptions, so I carried that over here.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7505104
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> + * Instances are tied to Availability Zones. However, the instance
> + * ID is tied to the Region.
> + * @param filter
> + *
> + * @see #runInstancesInRegion
> + * @see #terminateInstancesInRegion
> + * @see <a href="http://docs.amazonwebservices.com/AWSEC2/latest/APIReference/ApiReference-query-DescribeInstances.html"
> + * />
> + */
> + @Named("DescribeInstances")
> + @POST
> + @Path("/")
> + @FormParams(keys = ACTION, values = "DescribeInstances")
> + @XMLResponseParser(AWSDescribeInstancesResponseHandler.class)
> + @Fallback(EmptySetOnNotFoundOr404.class)
> + Set<? extends Reservation<? extends AWSRunningInstance>> describeInstancesInRegionWithFilter(
@abayer: Thanks!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7387921
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> + void testDescribeSnapshotsWithFilter() {
> + String region = defaultRegion;
> + SortedSet<Snapshot> allResults = Sets.newTreeSet(client.describeSnapshotsInRegion(region));
> + assertNotNull(allResults);
> + if (allResults.size() >= 1) {
> + Snapshot snapshot = allResults.last();
> + Snapshot result = Iterables.getOnlyElement(client.describeSnapshotsInRegionWithFilter(region,
> + ImmutableMultimap.<String, String>builder()
> + .put("snapshot-id", snapshot.getId()).build()));
> + assertNotNull(result);
> + assertEquals(result, snapshot);
> + }
> + }
> +
> + @Test(expectedExceptions = AWSResponseException.class)
> + void testDescribeSnapshotsWithFilterInvalid() {
Done.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7508524
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
@demobox - if/when you get a chance, I'd appreciate your eyeballs on this. =)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-27820104
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> + * Instances are tied to Availability Zones. However, the instance
> + * ID is tied to the Region.
> + * @param filter
> + *
> + * @see #runInstancesInRegion
> + * @see #terminateInstancesInRegion
> + * @see <a href="http://docs.amazonwebservices.com/AWSEC2/latest/APIReference/ApiReference-query-DescribeInstances.html"
> + * />
> + */
> + @Named("DescribeInstances")
> + @POST
> + @Path("/")
> + @FormParams(keys = ACTION, values = "DescribeInstances")
> + @XMLResponseParser(AWSDescribeInstancesResponseHandler.class)
> + @Fallback(EmptySetOnNotFoundOr404.class)
> + Set<? extends Reservation<? extends AWSRunningInstance>> filterInstancesInRegion(
Any reason why this is "filterInstancesInRegion" when e.g. the response parser is "DescribeInstances"?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7360883
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #824](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/824/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-27610917
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #398](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/398/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-28511339
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> + assertNotNull(allResults);
> + if (allResults.size() >= 1) {
> + final SecurityGroup group = getLast(allResults);
> + // in case there are multiple groups with the same name, which is the case with VPC
> + ImmutableSet<SecurityGroup> expected = FluentIterable.from(allResults)
> + .filter(new Predicate<SecurityGroup>() {
> + @Override
> + public boolean apply(SecurityGroup in) {
> + return group.getName().equals(in.getName());
> + }
> + }).toSet();
> + ImmutableSet<SecurityGroup> result = ImmutableSet.copyOf(client.describeSecurityGroupsInRegionWithFilter(region,
> + ImmutableMultimap.<String, String>builder()
> + .put("group-name", group.getName()).build()));
> + // the above command has a chance of returning less groups than the original
> + assertTrue(expected.containsAll(result));
Done.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7508646
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> @@ -58,4 +62,38 @@ void testDescribeInstances() {
> assert allResults.size() >= 0 : allResults.size();
> }
> }
> -}
> +
> + @Test
> + void testFilterInstances() {
> + for (String region : view.unwrapApi(EC2Api.class).getAvailabilityZoneAndRegionApi().get().describeRegions().keySet()) {
> + Set<? extends Reservation<? extends RunningInstance>> allResults = client.describeInstancesInRegion(region);
> + assertNotNull(allResults);
> + assert allResults.size() >= 0 : allResults.size();
Use TestNG assertions?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7486488
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> @@ -92,6 +94,44 @@ void testDescribeSpotRequestsInRegion() {
> }
>
> @Test
> + void testDescribeSpotRequestsInRegionFilter() {
Dun.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7508704
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #367](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/367/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-27727413
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> + .build());
> + assertNotNull(filterResult);
> + assertEquals(filterResult.size(), 1);
> + iterator = filterResult.iterator();
> + assertEquals(iterator.next().getId(), id1);
> + }
> + }
> +
> + @Test(expectedExceptions = AWSResponseException.class)
> + public void testDescribeImagesWithInvalidFilter() {
> + // Just run in the first region - no need to take the time on all of them.
> + String region = getFirst(ec2Api.getConfiguredRegions(), null);
> + if (region != null) {
> + Set<? extends Image> allResults = client.describeImagesInRegion(region);
> + assertNotNull(allResults);
> + assert allResults.size() >= 2 : allResults.size();
Done.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7508428
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> @@ -66,6 +69,39 @@
> @EndpointParam(parser = RegionToEndpointOrProviderIfNull.class) @Nullable String region,
> @BinderParam(BindInstanceIdsToIndexedFormParams.class) String... instanceIds);
>
> + /**
> + * Returns information about instances that you own.
> + * <p/>
> + *
> + * If you specify one or filters, Amazon EC2 returns information for instances
> + * matching those filters. If you do not specify any filters, Amazon EC2
> + * returns information for all relevant instances. If you specify an invalid
> + * filter, a fault is returned. Only instances you own will be included in the
> + * results.
> + * <p/>
> + * Recently terminated instances might appear in the returned results.This
[minor] space before "This"
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7360865
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> + void testDescribeSnapshotsWithFilter() {
> + String region = defaultRegion;
> + SortedSet<Snapshot> allResults = Sets.newTreeSet(client.describeSnapshotsInRegion(region));
> + assertNotNull(allResults);
> + if (allResults.size() >= 1) {
> + Snapshot snapshot = allResults.last();
> + Snapshot result = Iterables.getOnlyElement(client.describeSnapshotsInRegionWithFilter(region,
> + ImmutableMultimap.<String, String>builder()
> + .put("snapshot-id", snapshot.getId()).build()));
> + assertNotNull(result);
> + assertEquals(result, snapshot);
> + }
> + }
> +
> + @Test(expectedExceptions = AWSResponseException.class)
> + void testDescribeSnapshotsWithFilterInvalid() {
[minor] "WithInvalidFilter"?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7486463
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> @@ -132,11 +138,55 @@ public void testDescribeImages() {
> }
>
> @Test
> + public void testDescribeImagesWithFilter() {
> + // Just run in the first region - no need to take the time on all of them.
> + String region = getFirst(ec2Api.getConfiguredRegions(), null);
> + if (region != null) {
> + Set<? extends Image> allResults = client.describeImagesInRegion(region);
> + assertNotNull(allResults);
> + assert allResults.size() >= 2 : allResults.size();
> + Iterator<? extends Image> iterator = allResults.iterator();
[minor] inline here and below?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7486409
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> +import org.jclouds.aws.ec2.AWSEC2Api;
> +import org.jclouds.aws.ec2.compute.internal.BaseAWSEC2ComputeServiceExpectTest;
> +import org.jclouds.aws.ec2.domain.AWSRunningInstance;
> +import org.jclouds.http.HttpRequest;
> +import org.jclouds.http.HttpResponse;
> +import org.jclouds.util.Strings2;
> +import org.testng.annotations.Test;
> +
> +import com.google.common.collect.ImmutableMultimap;
> +import com.google.common.collect.ImmutableSet;
> +
> +/**
> + * @author Andrew Bayer
> + */
> +@Test(groups = "unit")
> +public class AWSInstanceApiExpectTest extends BaseAWSEC2ComputeServiceExpectTest {
Uh...beats me? I thought we were supposed to be using expect tests still - ec2/aws-ec2 is full of old code not using expect tests so I'm having to start from scratch in some places, but everything else I've been doing in recent months (GCE, etc) has been expect tests.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7360956
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> .addFormParam("SignatureMethod", "HmacSHA256")
> .addFormParam("SignatureVersion", "2")
> .addFormParam("Timestamp", "2009-11-08T15%3A54%3A08.897Z")
> .addFormParam("UserGroup.1", "all")
> .addFormParam("UserId.1", "bob")
> .addFormParam("UserId.2", "sue")
> - .addFormParam("Version", "2010-06-15")
> + .addFormParam("Version", "2010-08-31")
Hmmm...yeah, lemme think about that. May do that in a different PR, though, across the board.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7508466
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> @@ -58,4 +62,38 @@ void testDescribeInstances() {
> assert allResults.size() >= 0 : allResults.size();
> }
> }
> -}
> +
> + @Test
> + void testFilterInstances() {
> + for (String region : view.unwrapApi(EC2Api.class).getAvailabilityZoneAndRegionApi().get().describeRegions().keySet()) {
> + Set<? extends Reservation<? extends RunningInstance>> allResults = client.describeInstancesInRegion(region);
> + assertNotNull(allResults);
> + assert allResults.size() >= 0 : allResults.size();
> +
> + if (!allResults.isEmpty()) {
Yeah - if you have no instances in a region, then there's nothing to filter against.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7508595
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> +
> + assertEquals(instance.getId(), "i-059c2564");
> + }
> +
> + public void testFilterWhenResponseIs404() throws Exception {
> +
> + HttpResponse filterResponse = HttpResponse.builder().statusCode(404).build();
> +
> + AWSEC2Api apiWhenDontExist = requestsSendResponses(describeRegionsRequest, describeRegionsResponse,
> + filter, filterResponse).getContext().unwrapApi(AWSEC2Api.class);
> +
> + assertEquals(apiWhenDontExist.getInstanceApi().get().filterInstancesInRegion("us-east-1",
> + ImmutableMultimap.<String, String> builder()
> + .put("key-name", "jclouds#ec2-e#us-east-1#85")
> + .build()), ImmutableSet.of());
> + }
Probably. I need to do some live testing.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7360958
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> + Set<? extends Image> filterResult = client.describeImagesInRegionWithFilter(region,
> + ImmutableMultimap.<String, String>builder()
> + .put("image-id", id1)
> + .build());
> + assertNotNull(filterResult);
> + assertEquals(filterResult.size(), 1);
> + iterator = filterResult.iterator();
> + assertEquals(iterator.next().getId(), id1);
> + }
> + }
> +
> + @Test(expectedExceptions = AWSResponseException.class)
> + public void testDescribeImagesWithInvalidFilter() {
> + // Just run in the first region - no need to take the time on all of them.
> + String region = getFirst(ec2Api.getConfiguredRegions(), null);
> + if (region != null) {
Especially since we're expecting an exception here...should the test not fail explicitly if no regions are configured?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7486417
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> @@ -66,6 +69,39 @@
> @EndpointParam(parser = RegionToEndpointOrProviderIfNull.class) @Nullable String region,
> @BinderParam(BindInstanceIdsToIndexedFormParams.class) String... instanceIds);
>
> + /**
> + * Returns information about instances that you own.
> + * <p/>
> + *
> + * If you specify one or filters, Amazon EC2 returns information for instances
> + * matching those filters. If you do not specify any filters, Amazon EC2
> + * returns information for all relevant instances. If you specify an invalid
> + * filter, a fault is returned. Only instances you own will be included in the
Yes, exception is thrown. I'll doc it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7385522
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> @@ -110,6 +112,41 @@ void testDescribe() {
> }
>
> @Test
> + void testFilter() {
[minor] `public void`, as for most tests?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7486520
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
Note that this is still work-in-progress - once it's actually done I'll squash down, but I like having the CI/eyeballs on work-in-progress stuff. =)
I did bump the API version to 2010-08-31 - that, frankly, seems reasonable at this point. It's a 2.5 month API bump from where we were, it's over three years old, etc...but I'm open to criticism.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-27655010
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
I'd like to keep the varargs stuff, if that's alright, just so as not to break existing logic and the like.
I thought I'd caught the remaining .size() thingies in the new code, but I'll check again.
The tests that loop and only assert if they find regions and then fail...I think we've got enough other stuff that's gonna barf if we don't find any regions that I'm ok with leaving them as is.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-28507876
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> +
> + assertEquals(instance.getId(), "i-059c2564");
> + }
> +
> + public void testFilterWhenResponseIs404() throws Exception {
> +
> + HttpResponse filterResponse = HttpResponse.builder().statusCode(404).build();
> +
> + AWSEC2Api apiWhenDontExist = requestsSendResponses(describeRegionsRequest, describeRegionsResponse,
> + filter, filterResponse).getContext().unwrapApi(AWSEC2Api.class);
> +
> + assertEquals(apiWhenDontExist.getInstanceApi().get().filterInstancesInRegion("us-east-1",
> + ImmutableMultimap.<String, String> builder()
> + .put("key-name", "jclouds#ec2-e#us-east-1#85")
> + .build()), ImmutableSet.of());
> + }
Do we need a test case for "invalid filter"?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7360912
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> +import org.jclouds.aws.ec2.AWSEC2Api;
> +import org.jclouds.aws.ec2.compute.internal.BaseAWSEC2ComputeServiceExpectTest;
> +import org.jclouds.aws.ec2.domain.AWSRunningInstance;
> +import org.jclouds.http.HttpRequest;
> +import org.jclouds.http.HttpResponse;
> +import org.jclouds.util.Strings2;
> +import org.testng.annotations.Test;
> +
> +import com.google.common.collect.ImmutableMultimap;
> +import com.google.common.collect.ImmutableSet;
> +
> +/**
> + * @author Andrew Bayer
> + */
> +@Test(groups = "unit")
> +public class AWSInstanceApiExpectTest extends BaseAWSEC2ComputeServiceExpectTest {
Then I'm sure we'll live with this one using expect tests, too.
@adriancole: thoughts on preferred test style?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7361167
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> @@ -59,4 +62,27 @@ void testDescribeInstances() {
> }
> }
>
> + @Test
> + void testFilterInstances() {
> + for (String region : view.unwrapApi(AWSEC2Api.class).getAvailabilityZoneAndRegionApi().get().describeRegions().keySet()) {
> + Set<? extends Reservation<? extends RunningInstance>> allResults = client.describeInstancesInRegion(region);
> + assertNotNull(allResults);
> + assert allResults.size() >= 0 : allResults.size();
That one predated me - it's copied from testDescribeInstances above. Also, it does allow for the possibility of an empty set, if you don't have instances in one of the regions.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7360978
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #822](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-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/194#issuecomment-27540408
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> + public void testFilterWhenResponseIs2xx() throws Exception {
> +
> + HttpResponse filterResponse = HttpResponse.builder().statusCode(200)
> + .payload(payloadFromResourceWithContentType("/describe_instances_running.xml", "text/xml")).build();
> +
> +
> + EC2Api apiWhenExist = requestsSendResponses(describeRegionsRequest, describeRegionsResponse,
> + filter, filterResponse);
> +
> + RunningInstance instance = getOnlyElement(getOnlyElement(apiWhenExist.getInstanceApi().get().describeInstancesInRegionWithFilter("us-east-1",
> + ImmutableMultimap.<String, String>builder()
> + .put("key-name", "adriancole.ec21")
> + .build())));
> + assertNotNull(instance, "Instance should not be null");
> +
> + Assert.assertEquals(instance.getId(), "i-0799056f");
[minor] Static import?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7513163
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> + * @see #describeSnapshotsInRegion
> + * @see #createSnapshotsInRegion
> + * @see #deleteSnapshotInRegion
> + * @see <a href="http://docs.amazonwebservices.com/AWSEC2/latest/APIReference/ApiReference-query-DescribeSnapshots.html"
> + * />
> + */
> + @Named("DescribeSnapshots")
> + @POST
> + @Path("/")
> + @FormParams(keys = ACTION, values = "DescribeSnapshots")
> + @XMLResponseParser(DescribeSnapshotsResponseHandler.class)
> + @Fallback(EmptySetOnNotFoundOr404.class)
> + Set<Snapshot> describeSnapshotsInRegionWithFilter(
> + @EndpointParam(parser = RegionToEndpointOrProviderIfNull.class) @Nullable String region,
> + @BinderParam(BindFiltersToIndexedFormParams.class) Multimap<String, String> filter,
> + DescribeSnapshotsOptions... options);
Same thing as above with DescribeImagesOptions - this is aping how the describeSnapshotsInRegion method is set up.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7505135
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> ImmutableSet<SecurityGroup> result = ImmutableSet.copyOf(client.describeSecurityGroupsInRegion(region,
> - group.getName()));
> + group.getName()));
> + // the above command has a chance of returning less groups than the original
> + assertTrue(expected.containsAll(result));
Add message?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7486505
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> @@ -224,15 +226,23 @@ void deleteSecurityGroup(String region, String group) {
>
> @VisibleForTesting
> void deleteKeyPair(String region, String group) {
> - for (KeyPair keyPair : client.getKeyPairApi().get().describeKeyPairsInRegion(region)) {
> + for (KeyPair keyPair : client.getKeyPairApi().get().describeKeyPairsInRegionWithFilter(region,
> + ImmutableMultimap.<String, String>builder()
> + .put("key-name", Strings2.urlEncode(
> + String.format("jclouds#%s#%s*", group, region).replace('#', delimiter)))
Not entirely sure what the point of that is in the first place, but it was already there when I got there, as it were. Either way works for me.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7505087
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> + * @see InstanceApi#describeInstances
> + * @see #describeImageAttribute
> + * @see <a href="http://docs.amazonwebservices.com/AWSEC2/latest/APIReference/ApiReference-query-DescribeImages.html"
> + * />
> + * @see DescribeImagesOptions
> + */
> + @Named("DescribeImages")
> + @POST
> + @Path("/")
> + @FormParams(keys = ACTION, values = "DescribeImages")
> + @XMLResponseParser(DescribeImagesResponseHandler.class)
> + @Fallback(EmptySetOnNotFoundOr404.class)
> + Set<? extends Image> describeImagesInRegionWithFilter(
> + @EndpointParam(parser = RegionToEndpointOrProviderIfNull.class) @Nullable String region,
> + @BinderParam(BindFiltersToIndexedFormParams.class) Multimap<String, String> filter,
> + DescribeImagesOptions... options);
OK, then I'd ask the same question there ;-) It looks suspiciously like a copy-paste error from a `String... names` or similar method - how would you supply _multiple_ options objects? What would that even mean?
Could you remove the `...` and see if that breaks anything?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7513071
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> + * Instances are tied to Availability Zones. However, the instance
> + * ID is tied to the Region.
> + * @param filter
> + *
> + * @see #runInstancesInRegion
> + * @see #terminateInstancesInRegion
> + * @see <a href="http://docs.amazonwebservices.com/AWSEC2/latest/APIReference/ApiReference-query-DescribeInstances.html"
> + * />
> + */
> + @Named("DescribeInstances")
> + @POST
> + @Path("/")
> + @FormParams(keys = ACTION, values = "DescribeInstances")
> + @XMLResponseParser(AWSDescribeInstancesResponseHandler.class)
> + @Fallback(EmptySetOnNotFoundOr404.class)
> + Set<? extends Reservation<? extends AWSRunningInstance>> filterInstancesInRegion(
"describeInstancesInRegionWithFilter" or so would also work for me. "filterSomething" sounds to me like it might have some effect, e.g. removing things that don't match the filter, or so..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7361153
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> + Set<? extends Reservation<? extends RunningInstance>> allResults = client.describeInstancesInRegion(region);
> + assertNotNull(allResults);
> + assert allResults.size() >= 0 : allResults.size();
> +
> + if (allResults.size() > 0) {
> + RunningInstance instance = getFirst(getFirst(allResults, null), null);
> +
> + assertNotNull(instance);
> +
> + Set<? extends Reservation<? extends RunningInstance>> filterResults = client.filterInstancesInRegion(region,
> + ImmutableMultimap.<String, String>builder()
> + .put("key-name", instance.getKeyName())
> + .build());
> +
> + assertNotNull(filterResults);
> + assertTrue(filterResults.size() > 0, "No results found for filter, but there should be.");
`!filterResults.isEmpty()`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7360931
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> @@ -59,4 +62,27 @@ void testDescribeInstances() {
> }
> }
>
> + @Test
> + void testFilterInstances() {
> + for (String region : view.unwrapApi(AWSEC2Api.class).getAvailabilityZoneAndRegionApi().get().describeRegions().keySet()) {
> + Set<? extends Reservation<? extends RunningInstance>> allResults = client.describeInstancesInRegion(region);
> + assertNotNull(allResults);
> + assert allResults.size() >= 0 : allResults.size();
Oh, doh! Missed the subtle `=` difference between this one and the assertion below. But, erm, how can a set have a negative `size()` in the first place..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7361182
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
Cleaned up the asserts, the filter allResults tests, and the package-level voids. New commit incoming!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-28006485
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> jclouds-pull-requests #366 UNSTABLE
Spurious [test failure](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/org.apache.jclouds$jclouds-compute/366/testReport/junit/org.jclouds.compute.callables/BlockUntilInitScriptStatusIsZeroThenReturnOutputTest/testloopUntilTrueOrThrowCancellationExceptionReturnsWhenPredicateIsTrueSecondTimeWhileNotCancelled/)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-27707828
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #365](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/365/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-27660229
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> +import org.jclouds.rest.internal.BaseRestApiExpectTest;
> +import org.testng.annotations.Test;
> +
> +import com.google.common.collect.ImmutableMultimap;
> +import com.google.common.collect.ImmutableSet;
> +
> +/**
> + * @author Andrew Bayer
> + */
> +@Test(groups = "unit")
> +public class ElasticIPAddressApiExpectTest extends BaseEC2ApiExpectTest<EC2Api> {
> + /**
> + * @see org.jclouds.ec2.features.ElasticIPAddressApi
> + * @see org.jclouds.rest.annotations.SinceApiVersion
> + */
> + protected Properties setupProperties() {
Yup. Fixing as per below.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7508547
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
Modified a bit - had a couple screw-ups in the live tests, and I optimized some of the AMI-related tests to only look at one region (so as to save about 45 seconds of lookup time per region for no good reason) and to use the new filter methods for a couple things, for similar time savings. Also, aws-ec2 spot instance price history tests failed because there's apparently a new instance type, g2.2xlarge, at least in spot price history info, but not anywhere else I can find it. Fun! =)
But all told, this is good to go. All relevant live tests pass, nothing regresses in either ec2 or aws-ec2.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-27725922
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #862](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/862/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-28518420
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> @@ -59,4 +62,27 @@ void testDescribeInstances() {
> }
> }
>
> + @Test
> + void testFilterInstances() {
> + for (String region : view.unwrapApi(AWSEC2Api.class).getAvailabilityZoneAndRegionApi().get().describeRegions().keySet()) {
> + Set<? extends Reservation<? extends RunningInstance>> allResults = client.describeInstancesInRegion(region);
> + assertNotNull(allResults);
> + assert allResults.size() >= 0 : allResults.size();
That...I do not know. Cargo cult code here, so I make no claims to knowing exactly what it's doing. =)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7385249
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> + * @see InstanceApi#describeInstances
> + * @see #describeImageAttribute
> + * @see <a href="http://docs.amazonwebservices.com/AWSEC2/latest/APIReference/ApiReference-query-DescribeImages.html"
> + * />
> + * @see DescribeImagesOptions
> + */
> + @Named("DescribeImages")
> + @POST
> + @Path("/")
> + @FormParams(keys = ACTION, values = "DescribeImages")
> + @XMLResponseParser(DescribeImagesResponseHandler.class)
> + @Fallback(EmptySetOnNotFoundOr404.class)
> + Set<? extends Image> describeImagesInRegionWithFilter(
> + @EndpointParam(parser = RegionToEndpointOrProviderIfNull.class) @Nullable String region,
> + @BinderParam(BindFiltersToIndexedFormParams.class) Multimap<String, String> filter,
> + DescribeImagesOptions... options);
Ah, I think I understand this now...I'm guessing this is an easy way to allow "zero or one" Options objects without having to overload the method. I guess the core takes care of extracting the options object, if present, from the array. I assume that any Options objects beyond the first one would be ignored..?
@adriancole: Could you shed any light on this, if you have a moment?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7546889
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> +import org.jclouds.aws.ec2.AWSEC2Api;
> +import org.jclouds.aws.ec2.compute.internal.BaseAWSEC2ComputeServiceExpectTest;
> +import org.jclouds.aws.ec2.domain.AWSRunningInstance;
> +import org.jclouds.http.HttpRequest;
> +import org.jclouds.http.HttpResponse;
> +import org.jclouds.util.Strings2;
> +import org.testng.annotations.Test;
> +
> +import com.google.common.collect.ImmutableMultimap;
> +import com.google.common.collect.ImmutableSet;
> +
> +/**
> + * @author Andrew Bayer
> + */
> +@Test(groups = "unit")
> +public class AWSInstanceApiExpectTest extends BaseAWSEC2ComputeServiceExpectTest {
Are we trying to avoid expect tests and use mockWebServer tests now?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7360901
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> @@ -92,6 +94,44 @@ void testDescribeSpotRequestsInRegion() {
> }
>
> @Test
> + void testDescribeSpotRequestsInRegionFilter() {
`public void`, as usually for tests?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7486525
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #826](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/826/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-27656742
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> + * @see InstanceApi#describeInstances
> + * @see #describeImageAttribute
> + * @see <a href="http://docs.amazonwebservices.com/AWSEC2/latest/APIReference/ApiReference-query-DescribeImages.html"
> + * />
> + * @see DescribeImagesOptions
> + */
> + @Named("DescribeImages")
> + @POST
> + @Path("/")
> + @FormParams(keys = ACTION, values = "DescribeImages")
> + @XMLResponseParser(DescribeImagesResponseHandler.class)
> + @Fallback(EmptySetOnNotFoundOr404.class)
> + Set<? extends Image> describeImagesInRegionWithFilter(
> + @EndpointParam(parser = RegionToEndpointOrProviderIfNull.class) @Nullable String region,
> + @BinderParam(BindFiltersToIndexedFormParams.class) Multimap<String, String> filter,
> + DescribeImagesOptions... options);
Is this really supposed to be a varargs array of DescribeImagesOptions?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7486339
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #362](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/362/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-27656433
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> +import org.testng.Assert;
> +import org.testng.annotations.Test;
> +
> +import com.google.common.collect.ImmutableMultimap;
> +import com.google.common.collect.ImmutableSet;
> +
> +/**
> + * @author Andrew Bayer
> + */
> +@Test(groups = "unit")
> +public class InstanceApiExpectTest extends BaseEC2ApiExpectTest<EC2Api> {
> + /**
> + * @see InstanceApi
> + * @see org.jclouds.rest.annotations.SinceApiVersion
> + */
> + protected Properties setupProperties() {
Yup, fixed.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7508550
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> +import org.jclouds.rest.internal.BaseRestApiExpectTest;
> +import org.testng.annotations.Test;
> +
> +import com.google.common.collect.ImmutableMultimap;
> +import com.google.common.collect.ImmutableSet;
> +
> +/**
> + * @author Andrew Bayer
> + */
> +@Test(groups = "unit")
> +public class ElasticIPAddressApiExpectTest extends BaseEC2ApiExpectTest<EC2Api> {
> + /**
> + * @see org.jclouds.ec2.features.ElasticIPAddressApi
> + * @see org.jclouds.rest.annotations.SinceApiVersion
> + */
> + protected Properties setupProperties() {
Spurious Javadoc comment? Remove?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7486470
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> + * Instances are tied to Availability Zones. However, the instance
> + * ID is tied to the Region.
> + * @param filter
> + *
> + * @see #runInstancesInRegion
> + * @see #terminateInstancesInRegion
> + * @see <a href="http://docs.amazonwebservices.com/AWSEC2/latest/APIReference/ApiReference-query-DescribeInstances.html"
> + * />
> + */
> + @Named("DescribeInstances")
> + @POST
> + @Path("/")
> + @FormParams(keys = ACTION, values = "DescribeInstances")
> + @XMLResponseParser(AWSDescribeInstancesResponseHandler.class)
> + @Fallback(EmptySetOnNotFoundOr404.class)
> + Set<? extends Reservation<? extends AWSRunningInstance>> filterInstancesInRegion(
I decided not to overload describeInstancesInRegion, basically. It just felt a bit cleaner to me - that said, there'd be no problem with just making this another describeInstancesInRegion permutation.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7360933
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #861](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/861/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-28511375
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> + assertNotNull(allResults);
> + if (allResults.size() >= 1) {
> + final SecurityGroup group = getLast(allResults);
> + // in case there are multiple groups with the same name, which is the case with VPC
> + ImmutableSet<SecurityGroup> expected = FluentIterable.from(allResults)
> + .filter(new Predicate<SecurityGroup>() {
> + @Override
> + public boolean apply(SecurityGroup in) {
> + return group.getName().equals(in.getName());
> + }
> + }).toSet();
> + ImmutableSet<SecurityGroup> result = ImmutableSet.copyOf(client.describeSecurityGroupsInRegionWithFilter(region,
> + ImmutableMultimap.<String, String>builder()
> + .put("group-name", group.getName()).build()));
> + // the above command has a chance of returning less groups than the original
> + assertTrue(expected.containsAll(result));
Add message?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7486506
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> @@ -115,7 +119,9 @@ public void testDescribeImageBadId() {
> }
>
> public void testDescribeImages() {
> - for (String region : ec2Api.getConfiguredRegions()) {
> + // Just run in the first region - no need to take the time on all of them.
> + String region = getFirst(ec2Api.getConfiguredRegions(), null);
> + if (region != null) {
I...think so. I'd be fairly surprised if it wasn't, but I'm not strongly opposed to rolling back - I just got tired of waiting 4 minutes for this test (and its ilk) to run in all regions. =)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7505159
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> @@ -132,11 +138,55 @@ public void testDescribeImages() {
> }
>
> @Test
> + public void testDescribeImagesWithFilter() {
> + // Just run in the first region - no need to take the time on all of them.
> + String region = getFirst(ec2Api.getConfiguredRegions(), null);
> + if (region != null) {
If there are _no_ configured regions, shouldn't that flag up something rather than a silent test success? At least throw a `SkipException`, or something like that?
Comment also applied to some other tests, I think.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7486395
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> @@ -66,6 +69,39 @@
> @EndpointParam(parser = RegionToEndpointOrProviderIfNull.class) @Nullable String region,
> @BinderParam(BindInstanceIdsToIndexedFormParams.class) String... instanceIds);
>
> + /**
> + * Returns information about instances that you own.
> + * <p/>
> + *
> + * If you specify one or filters, Amazon EC2 returns information for instances
> + * matching those filters. If you do not specify any filters, Amazon EC2
> + * returns information for all relevant instances. If you specify an invalid
> + * filter, a fault is returned. Only instances you own will be included in the
Is "fault is returned" = "exception is thrown" here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7360859
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> @@ -75,16 +77,44 @@ public void setupContext() {
>
> @Test
> void testDescribeVolumes() {
> - for (String region : ec2Api.getConfiguredRegions()) {
> - SortedSet<Volume> allResults = Sets.newTreeSet(client.describeVolumesInRegion(region));
> - assertNotNull(allResults);
> - if (allResults.size() >= 1) {
> - Volume volume = allResults.last();
> - SortedSet<Volume> result = Sets.newTreeSet(client.describeVolumesInRegion(region, volume.getId()));
> - assertNotNull(result);
> - Volume compare = result.last();
> - assertEquals(compare, volume);
> - }
> + String region = defaultRegion;
> + SortedSet<Volume> allResults = Sets.newTreeSet(client.describeVolumesInRegion(region));
> + assertNotNull(allResults);
> + if (allResults.size() >= 1) {
`!allResults.isEmpty()`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7486434
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
> + * @see #describeSnapshotsInRegion
> + * @see #createSnapshotsInRegion
> + * @see #deleteSnapshotInRegion
> + * @see <a href="http://docs.amazonwebservices.com/AWSEC2/latest/APIReference/ApiReference-query-DescribeSnapshots.html"
> + * />
> + */
> + @Named("DescribeSnapshots")
> + @POST
> + @Path("/")
> + @FormParams(keys = ACTION, values = "DescribeSnapshots")
> + @XMLResponseParser(DescribeSnapshotsResponseHandler.class)
> + @Fallback(EmptySetOnNotFoundOr404.class)
> + Set<Snapshot> describeSnapshotsInRegionWithFilter(
> + @EndpointParam(parser = RegionToEndpointOrProviderIfNull.class) @Nullable String region,
> + @BinderParam(BindFiltersToIndexedFormParams.class) Multimap<String, String> filter,
> + DescribeSnapshotsOptions... options);
See response above
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7513083
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Phillips <no...@github.com>.
Thanks for the big cleanup, @abayer! Just a couple of comments:
* could we see what happens if we remove the weird `DescribeOptions...` varargs?
* still a bunch of possible `allResults.size() >= 1` -> `!allResults.isEmpty()` changes, but if you prefer the former, that's fine with me
* still quite a few tests that follow the pattern "loop across some regions, only run assertions if we find any results". Specifically, these tests look like they will succeed even if nothing is returned in any regions, and so _no_ assertions are ever carried out. Is that intentional?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-28014245
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> + Set<? extends Reservation<? extends RunningInstance>> allResults = client.describeInstancesInRegion(region);
> + assertNotNull(allResults);
> + assert allResults.size() >= 0 : allResults.size();
> +
> + if (allResults.size() > 0) {
> + RunningInstance instance = getFirst(getFirst(allResults, null), null);
> +
> + assertNotNull(instance);
> +
> + Set<? extends Reservation<? extends RunningInstance>> filterResults = client.filterInstancesInRegion(region,
> + ImmutableMultimap.<String, String>builder()
> + .put("key-name", instance.getKeyName())
> + .build());
> +
> + assertNotNull(filterResults);
> + assertTrue(filterResults.size() > 0, "No results found for filter, but there should be.");
durrr, yeah.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7360960
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
> + * Instances are tied to Availability Zones. However, the instance
> + * ID is tied to the Region.
> + * @param filter
> + *
> + * @see #runInstancesInRegion
> + * @see #terminateInstancesInRegion
> + * @see <a href="http://docs.amazonwebservices.com/AWSEC2/latest/APIReference/ApiReference-query-DescribeInstances.html"
> + * />
> + */
> + @Named("DescribeInstances")
> + @POST
> + @Path("/")
> + @FormParams(keys = ACTION, values = "DescribeInstances")
> + @XMLResponseParser(AWSDescribeInstancesResponseHandler.class)
> + @Fallback(EmptySetOnNotFoundOr404.class)
> + Set<? extends Reservation<? extends AWSRunningInstance>> filterInstancesInRegion(
okiedokie, done.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194/files#r7385247
Re: [jclouds] JCLOUDS-361. Add support for filter parameters to
DescribeInstances for aws-ec2 (#194)
Posted by Andrew Bayer <no...@github.com>.
fyi, I'm reworking this now to move the filter method into apis/ec2 rather than providers/aws-ec2 - it's a three year old feature in the EC2 api, so I think it's not unreasonable, especially given that I can flag SinceApiVersion and all. I'm also adding the describeFooInRegionWithFilter methods for AMIs, addresses, etc, and then calling it a day 'cos figuring out how to map nodemetadata predicates to filter multimaps is, well, sadly impossible. =) I may do another JIRA for adding new (action)NodesMatching methods to ComputeService that take multimaps if I can find other clouds with similar behavior, though.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/194#issuecomment-27629276