You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Andrea Turli <no...@github.com> on 2018/07/06 13:19:20 UTC
[jclouds/jclouds-labs] [JCLOUDS-1430] - add more features (#441)
- add securitygroup-api
- add keypair-api
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-labs/pull/441
-- Commit Summary --
* [JCLOUDS-1430] - add more features
-- File Changes --
M aliyun-ecs/src/main/java/org/jclouds/aliyun/ecs/ECSComputeServiceApi.java (7)
A aliyun-ecs/src/main/java/org/jclouds/aliyun/ecs/domain/IpProtocol.java (31)
A aliyun-ecs/src/main/java/org/jclouds/aliyun/ecs/domain/KeyPair.java (41)
A aliyun-ecs/src/main/java/org/jclouds/aliyun/ecs/domain/KeyPairRequest.java (76)
A aliyun-ecs/src/main/java/org/jclouds/aliyun/ecs/domain/KeyPairs.java (33)
A aliyun-ecs/src/main/java/org/jclouds/aliyun/ecs/domain/Permission.java (76)
A aliyun-ecs/src/main/java/org/jclouds/aliyun/ecs/domain/Request.java (58)
A aliyun-ecs/src/main/java/org/jclouds/aliyun/ecs/domain/SecurityGroup.java (42)
A aliyun-ecs/src/main/java/org/jclouds/aliyun/ecs/domain/SecurityGroupRequest.java (59)
A aliyun-ecs/src/main/java/org/jclouds/aliyun/ecs/domain/SecurityGroups.java (33)
A aliyun-ecs/src/main/java/org/jclouds/aliyun/ecs/domain/options/CreateSecurityGroupOptions.java (78)
A aliyun-ecs/src/main/java/org/jclouds/aliyun/ecs/domain/options/DeleteKeyPairOptions.java (51)
A aliyun-ecs/src/main/java/org/jclouds/aliyun/ecs/domain/options/ListKeyPairsOptions.java (64)
A aliyun-ecs/src/main/java/org/jclouds/aliyun/ecs/domain/options/ListSecurityGroupsOptions.java (62)
A aliyun-ecs/src/main/java/org/jclouds/aliyun/ecs/domain/options/TagOptions.java (127)
A aliyun-ecs/src/main/java/org/jclouds/aliyun/ecs/features/SecurityGroupApi.java (135)
A aliyun-ecs/src/main/java/org/jclouds/aliyun/ecs/features/SshKeyPairApi.java (127)
A aliyun-ecs/src/test/java/org/jclouds/aliyun/ecs/compute/features/SecurityGroupApiLiveTest.java (95)
A aliyun-ecs/src/test/java/org/jclouds/aliyun/ecs/compute/features/SecurityGroupApiMockTest.java (70)
A aliyun-ecs/src/test/java/org/jclouds/aliyun/ecs/compute/features/SshKeyPairApiLiveTest.java (84)
A aliyun-ecs/src/test/java/org/jclouds/aliyun/ecs/compute/features/SshKeyPairApiMockTest.java (108)
M aliyun-ecs/src/test/java/org/jclouds/aliyun/ecs/compute/internal/BaseECSComputeServiceApiMockTest.java (13)
A aliyun-ecs/src/test/resources/keypair-create-res.json (6)
A aliyun-ecs/src/test/resources/keypair-delete-res.json (3)
A aliyun-ecs/src/test/resources/keypair-import-res.json (6)
A aliyun-ecs/src/test/resources/keypairs-first.json (50)
A aliyun-ecs/src/test/resources/keypairs-last.json (18)
A aliyun-ecs/src/test/resources/securitygroups-first.json (61)
A aliyun-ecs/src/test/resources/securitygroups-last.json (31)
-- Patch Links --
https://github.com/jclouds/jclouds-labs/pull/441.patch
https://github.com/jclouds/jclouds-labs/pull/441.diff
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/441
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] - add more features (#441)
Posted by Andrea Turli <no...@github.com>.
andreaturli commented on this pull request.
> + * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jclouds.aliyun.ecs.domain;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Objects;
+
+import java.beans.ConstructorProperties;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+public class KeyPairRequest extends Request {
I had this one in mind https://github.com/google/auto/issues/277 but maybe it's resolved since 1.3.0+
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/441#discussion_r203162485
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] - add more features (#441)
Posted by Ignasi Barrera <no...@github.com>.
nacx approved this pull request.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/441#pullrequestreview-142069469
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] - add more features (#441)
Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.
> ICMP, GRE, TCP, UDP, ALL;
+ public static IpProtocol fromValue(String value) {
+ Optional<IpProtocol> ipProtocol = Enums.getIfPresent(IpProtocol.class, value.toUpperCase());
+ checkArgument(ipProtocol.isPresent(), "Expected one of %s but was %s", Joiner.on(',').join(Permission.Policy.values()), value);
`IpProtocol.values()`
> @AutoValue
public abstract class Permission {
- Permission() {
+ public enum NicType {
+ INTERNET, INTRANET;
+
+ public static NicType fromValue(String value) {
+ Optional<NicType> nicType = Enums.getIfPresent(NicType.class, value.toUpperCase());
+ checkArgument(nicType.isPresent(), "Expected one of %s but was %s", Joiner.on(',').join(Policy.values()), value);
`NicType.values()`
> + public enum Policy {
+ ACCEPT, DROP;
+
+ public static Policy fromValue(String value) {
+ Optional<Policy> policy = Enums.getIfPresent(Policy.class, value.toUpperCase());
+ checkArgument(policy.isPresent(), "Expected one of %s but was %s", Joiner.on(',').join(Policy.values()), value);
+ return policy.get();
+ }
+ }
+
+ public enum Direction {
+ EGRESS, ALL;
+
+ public static Direction fromValue(String value) {
+ Optional<Direction> direction = Enums.getIfPresent(Direction.class, value.toUpperCase());
+ checkArgument(direction.isPresent(), "Expected one of %s but was %s", Joiner.on(',').join(Policy.values()), value);
`Direction.values()`
>
+ private void validateInput(final String input, int maxLength) {
+ checkNotNull(input);
+ checkState(input.length() <= maxLength, String.format("input must be <= %d chars", maxLength));
+ checkState(Iterables.any(FORBIDDEN_PREFIX, new Predicate<String>() {
Negate the condition, otherwise you are failing if the value is correct.
> @@ -88,13 +99,19 @@
}
@Override
- protected Function<Object, IterableWithMarker<Image>> markerToNextForArg0(final Optional<Object> arg0) {
+ protected Function<Object, IterableWithMarker<Image>> markerToNextForArgs(final List<Object> args) {
+ if (args == null || args.isEmpty()) throw new IllegalStateException("Can't advance the PagedIterable");
+ final String regionId = args.get(0).toString();
+ final Optional<Object> optionalPaginationOptions = Optional.fromNullable(!args.isEmpty() && args.size() == 2 ? args.get(1) : null);
You could better use the `instanceOf` method from the `Predicates` class for a more safe approach.
> Request remove(@QueryParam("RegionId") String region,
@QueryParam("ResourceId") String resourceId,
@QueryParam("ResourceType") String resourceType);
+
+ @Named("tag:remove")
+ @POST
+ @QueryParams(keys = "Action", values = "RemoveTags")
+ Request remove(@QueryParam("RegionId") String region,
+ @QueryParam("ResourceId") String resourceId,
+ @QueryParam("ResourceType") String resourceType,
+ TagOptions... options);
This should not be a vararg argument.
> +import com.google.common.collect.Iterables;
+
+import javax.inject.Singleton;
+import java.util.Arrays;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+
+/**
+ * Takes an array of string and return a "["s1", "s2", … "sN"]"
+ */
+@Singleton
+public class ArrayToCommaSeparatedString implements Function<Object, String> {
+ @Override
+ public String apply(Object input) {
+ checkArgument(checkNotNull(input, "input") instanceof String[], "This function is only valid for array of Strings!");
Why do this? Why not just making this a `Function<String[], String>`? Casting inside and having a non-accurate signature seems to be wrong.
> +@Singleton
+public class ArrayToCommaSeparatedString implements Function<Object, String> {
+ @Override
+ public String apply(Object input) {
+ checkArgument(checkNotNull(input, "input") instanceof String[], "This function is only valid for array of Strings!");
+ String[] names = (String[]) input;
+
+ String arrayToCommaSeparatedString = Joiner.on(",")
+ .join(Iterables.transform(Arrays.asList(names), new Function<String, String>() {
+ @Override
+ public String apply(String s) {
+ return new StringBuilder(s.length() + 1).append('"').append(s).append('"').toString();
+ }
+ }));
+ return String.format("[%s]", arrayToCommaSeparatedString);
+ }
This class needs proper unit tests.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/441#pullrequestreview-141377515
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] - add more features (#441)
Posted by Andrea Turli <no...@github.com>.
andreaturli commented on this pull request.
> + @POST
+ @QueryParams(keys = "Action", values = "ImportKeyPair")
+ KeyPair importKeyPair(@QueryParam("RegionId") String region,
+ @QueryParam("PublicKeyBody") String publicKeyBody,
+ @QueryParam("KeyPairName") String keyPairName);
+
+ @Named("sshKeyPair:delete")
+ @POST
+ @QueryParams(keys = "Action", values = "DeleteKeyPairs")
+ Request delete(@QueryParam("RegionId") String region);
+
+ @Named("sshKeyPair:delete")
+ @POST
+ @QueryParams(keys = "Action", values = "DeleteKeyPairs")
+ Request delete(@QueryParam("RegionId") String region, DeleteKeyPairOptions deleteOptions);
+}
you mean `delete`? yes one or more. Have changed it as names are not optionals
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/441#discussion_r203304248
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] - add more features (#441)
Posted by Andrea Turli <no...@github.com>.
merging now
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/441#issuecomment-409340097
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] - add more features (#441)
Posted by Andrea Turli <no...@github.com>.
andreaturli commented on this pull request.
> + static class ToPagedIterable extends Arg0ToPagedIterable<SecurityGroup, ParseSecurityGroups.ToPagedIterable> {
+
+ private final ECSComputeServiceApi api;
+
+ @Inject
+ ToPagedIterable(ECSComputeServiceApi api) {
+ this.api = api;
+ }
+
+ @Override
+ protected Function<Object, IterableWithMarker<SecurityGroup>> markerToNextForArg0(final Optional<Object> arg0) {
+ return new Function<Object, IterableWithMarker<SecurityGroup>>() {
+ @Override
+ public IterableWithMarker<SecurityGroup> apply(Object input) {
+ String regionId = arg0.get().toString();
+ ListSecurityGroupsOptions listSecurityGroupsOptions = ListSecurityGroupsOptions.Builder.paginationOptions(PaginationOptions.class.cast(input));
ok refactored
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/441#discussion_r203304329
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] - add more features (#441)
Posted by Andrea Turli <no...@github.com>.
Closed #441.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/441#event-1763661946
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] - add more features (#441)
Posted by Andrea Turli <no...@github.com>.
@andreaturli pushed 1 commit.
4547b1b wip
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/441/files/f2e09f774d0d5236589bb1d2c075a887f2afaf43..4547b1b80cc47e1cfabac71e8c1747cd075cbc5b
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] - add more features (#441)
Posted by Andrea Turli <no...@github.com>.
andreaturli commented on this pull request.
> + return new Function<Object, IterableWithMarker<Tag>>() {
+ @Override
+ public IterableWithMarker<Tag> apply(Object input) {
+ String regionId = arg0.get().toString();
+ ListTagsOptions listTagsOptions = ListTagsOptions.Builder.paginationOptions(PaginationOptions.class.cast(input));
+ return api.tagApi().list(regionId, listTagsOptions);
+ }
+ };
+ }
+ }
+ }
+
+ @Named("tag:add")
+ @POST
+ @QueryParams(keys = "Action", values = "AddTags")
+ @Fallback(Fallbacks.EmptyListOnNotFoundOr404.class)
ok
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/441#discussion_r203304737
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] - add more features (#441)
Posted by Andrea Turli <no...@github.com>.
andreaturli commented on this pull request.
> + * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jclouds.aliyun.ecs.domain;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Objects;
+
+import java.beans.ConstructorProperties;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+public class KeyPairRequest extends Request {
https://github.com/google/auto/blob/master/value/userguide/howto.md#-have-one-autovalue-class-extend-another apparently we can't use `AutoValue` here
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/441#discussion_r203262402
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] - add more features (#441)
Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.
> @@ -84,7 +84,7 @@ public static TagOptions keys(Set<String> keys) {
private void validateInput(final String input, int maxLength) {
checkNotNull(input);
checkState(input.length() <= maxLength, String.format("input must be <= %d chars", maxLength));
- checkState(Iterables.any(FORBIDDEN_PREFIX, new Predicate<String>() {
+ checkState(Iterables.contains(FORBIDDEN_PREFIX, new Predicate<String>() {
Hmmmm If I understand this correctly, it is still not right.
You want to check that the given input **does not** contain any of the forbidden inputs, right? It should be something like>
```java
checkState(!Iterables.any(FORBIDDEN_PREFIX, new Predicate<String>() {
```
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/441#pullrequestreview-142055089
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] - add more features (#441)
Posted by Andrea Turli <no...@github.com>.
andreaturli commented on this pull request.
> +import com.google.common.collect.Iterables;
+
+import javax.inject.Singleton;
+import java.util.Arrays;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+
+/**
+ * Takes an array of string and return a "["s1", "s2", … "sN"]"
+ */
+@Singleton
+public class ArrayToCommaSeparatedString implements Function<Object, String> {
+ @Override
+ public String apply(Object input) {
+ checkArgument(checkNotNull(input, "input") instanceof String[], "This function is only valid for array of Strings!");
Agreed but I think it is because of `@ParamParser` annotation
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/441#discussion_r206304705
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] - add more features (#441)
Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.
First review done (not checked the tests).
I'll go through the tests review once the current comments are addressed.
> + * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jclouds.aliyun.ecs.domain;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Objects;
+
+import java.beans.ConstructorProperties;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+public class KeyPairRequest extends Request {
Use auto-value
> + public static Permission create(String sourceCidrIp, String destCidrIp, String description, String nicType,
+ String destGroupName, String portRange, String destGroupId, String direction, String priority,
+ String ipProtocol, String sourceGroupOwnerAccount, String policy, Date createTime, String sourceGroupId,
+ String destGroupOwnerAccount, String sourceGroupName) {
+ return new AutoValue_Permission(sourceCidrIp, destCidrIp, description, nicType, destGroupName, portRange,
+ destGroupId, direction, priority, ipProtocol, sourceGroupOwnerAccount, policy, createTime, sourceGroupId,
+ destGroupOwnerAccount, sourceGroupName);
+ }
+
+ public abstract String sourceCidrIp();
+
+ public abstract String destCidrIp();
+
+ public abstract String description();
+
+ public abstract String nicType();
Looks like a candidate for an enum?
> +
+ public abstract String sourceCidrIp();
+
+ public abstract String destCidrIp();
+
+ public abstract String description();
+
+ public abstract String nicType();
+
+ public abstract String destGroupName();
+
+ public abstract String portRange();
+
+ public abstract String destGroupId();
+
+ public abstract String direction();
Same here, does this have a finite set of possible values?
> +
+ public abstract String description();
+
+ public abstract String nicType();
+
+ public abstract String destGroupName();
+
+ public abstract String portRange();
+
+ public abstract String destGroupId();
+
+ public abstract String direction();
+
+ public abstract String priority();
+
+ public abstract String ipProtocol();
Same here about enums
> + * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jclouds.aliyun.ecs.domain;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Objects;
+
+import java.beans.ConstructorProperties;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+public class Request {
Use auto-value
> +import com.google.auto.value.AutoValue;
+import org.jclouds.json.SerializedNames;
+
+@AutoValue
+public abstract class SecurityGroup {
+
+ SecurityGroup() {
+ }
+
+ @SerializedNames({ "SecurityGroupId", "Description", "SecurityGroupName", "VpcId" })
+ public static SecurityGroup create(String securityGroupId, String description, String securityGroupName,
+ String vpcId) {
+ return new AutoValue_SecurityGroup(securityGroupId, description, securityGroupName, vpcId);
+ }
+
+ public abstract String securityGroupId();
In general, for all classes, remove the "securityGroup" prefix. Use just "id", "name". This class is already a Security group, so this kind of prefixes don't make sense.
> + * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jclouds.aliyun.ecs.domain;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Objects;
+
+import java.beans.ConstructorProperties;
+
+public class SecurityGroupRequest extends Request {
Why not auto-value?
> +import org.jclouds.http.options.BaseHttpRequestOptions;
+
+public class TagOptions extends BaseHttpRequestOptions {
+
+ public static final String TAG_1_KEY_PARAM = "Tag.1.Key";
+ public static final String TAG_2_KEY_PARAM = "Tag.2.Key";
+ public static final String TAG_3_KEY_PARAM = "Tag.3.Key";
+ public static final String TAG_4_KEY_PARAM = "Tag.4.Key";
+ public static final String TAG_5_KEY_PARAM = "Tag.5.Key";
+ public static final String TAG_1_VALUE_PARAM = "Tag.1.Value";
+ public static final String TAG_2_VALUE_PARAM = "Tag.2.Value";
+ public static final String TAG_3_VALUE_PARAM = "Tag.3.Value";
+ public static final String TAG_4_VALUE_PARAM = "Tag.4.Value";
+ public static final String TAG_5_VALUE_PARAM = "Tag.5.Value";
+
+ public TagOptions tag1Key(String tag1Key) {
Wouldn't it be better to just provide one method like this?
```java
public TagOptions tag(int pos, String name, String value)
```
> + @Override
+ public IterableWithMarker<SecurityGroup> apply(Object input) {
+ String regionId = arg0.get().toString();
+ ListSecurityGroupsOptions listSecurityGroupsOptions = ListSecurityGroupsOptions.Builder.paginationOptions(PaginationOptions.class.cast(input));
+ return api.securityGroupApi().list(regionId, listSecurityGroupsOptions);
+ }
+ };
+ }
+ }
+ }
+
+ @Named("securityGroup:get")
+ @GET
+ @QueryParams(keys = "Action", values = "DescribeSecurityGroupAttribute")
+ @SelectJson("Permission")
+ List<Permission> get(@QueryParam("RegionId") String region, @QueryParam("SecurityGroupId") String securityGroupId);
Returns a list?
> + }
+ };
+ }
+ }
+ }
+
+ @Named("securityGroup:get")
+ @GET
+ @QueryParams(keys = "Action", values = "DescribeSecurityGroupAttribute")
+ @SelectJson("Permission")
+ List<Permission> get(@QueryParam("RegionId") String region, @QueryParam("SecurityGroupId") String securityGroupId);
+
+ @Named("securityGroup:create")
+ @POST
+ @QueryParams(keys = "Action", values = "CreateSecurityGroup")
+ @Fallback(Fallbacks.EmptyListOnNotFoundOr404.class)
Remove this fallback. Don't use 4xx fallbacks in POST/PUT
> + @Named("securityGroup:get")
+ @GET
+ @QueryParams(keys = "Action", values = "DescribeSecurityGroupAttribute")
+ @SelectJson("Permission")
+ List<Permission> get(@QueryParam("RegionId") String region, @QueryParam("SecurityGroupId") String securityGroupId);
+
+ @Named("securityGroup:create")
+ @POST
+ @QueryParams(keys = "Action", values = "CreateSecurityGroup")
+ @Fallback(Fallbacks.EmptyListOnNotFoundOr404.class)
+ SecurityGroupRequest create(@QueryParam("RegionId") String region);
+
+ @Named("securityGroup:create")
+ @POST
+ @QueryParams(keys = "Action", values = "CreateSecurityGroup")
+ @Fallback(Fallbacks.EmptyListOnNotFoundOr404.class)
Remove the fallback.
> + @GET
+ @QueryParams(keys = "Action", values = "DescribeSecurityGroupAttribute")
+ @SelectJson("Permission")
+ List<Permission> get(@QueryParam("RegionId") String region, @QueryParam("SecurityGroupId") String securityGroupId);
+
+ @Named("securityGroup:create")
+ @POST
+ @QueryParams(keys = "Action", values = "CreateSecurityGroup")
+ @Fallback(Fallbacks.EmptyListOnNotFoundOr404.class)
+ SecurityGroupRequest create(@QueryParam("RegionId") String region);
+
+ @Named("securityGroup:create")
+ @POST
+ @QueryParams(keys = "Action", values = "CreateSecurityGroup")
+ @Fallback(Fallbacks.EmptyListOnNotFoundOr404.class)
+ SecurityGroupRequest create(@QueryParam("RegionId") String region, CreateSecurityGroupOptions options);
Options will be bound to the request as query params. Is that correct for this POST operation?
> + @Named("securityGroup:create")
+ @POST
+ @QueryParams(keys = "Action", values = "CreateSecurityGroup")
+ @Fallback(Fallbacks.EmptyListOnNotFoundOr404.class)
+ SecurityGroupRequest create(@QueryParam("RegionId") String region);
+
+ @Named("securityGroup:create")
+ @POST
+ @QueryParams(keys = "Action", values = "CreateSecurityGroup")
+ @Fallback(Fallbacks.EmptyListOnNotFoundOr404.class)
+ SecurityGroupRequest create(@QueryParam("RegionId") String region, CreateSecurityGroupOptions options);
+
+ @Named("securityGroup:addInbound")
+ @POST
+ @QueryParams(keys = "Action", values = "AuthorizeSecurityGroup")
+ @Fallback(Fallbacks.EmptyListOnNotFoundOr404.class)
Remove fallback.
> + @QueryParams(keys = "Action", values = "CreateSecurityGroup")
+ @Fallback(Fallbacks.EmptyListOnNotFoundOr404.class)
+ SecurityGroupRequest create(@QueryParam("RegionId") String region, CreateSecurityGroupOptions options);
+
+ @Named("securityGroup:addInbound")
+ @POST
+ @QueryParams(keys = "Action", values = "AuthorizeSecurityGroup")
+ @Fallback(Fallbacks.EmptyListOnNotFoundOr404.class)
+ Request addInboundRule(@QueryParam("RegionId") String region, @QueryParam("SecurityGroupId") String securityGroupId,
+ @QueryParam("IpProtocol") IpProtocol ipProtocol, @QueryParam("PortRange") String portRange,
+ @QueryParam("SourceCidrIp") String sourceCidrIp);
+
+ @Named("securityGroup:delete")
+ @POST
+ @QueryParams(keys = "Action", values = "DeleteSecurityGroup")
+ @Fallback(Fallbacks.EmptyListOnNotFoundOr404.class)
This does not return a list. Fix fallback.
> + @Named("sshKeyPair:create")
+ @POST
+ @QueryParams(keys = "Action", values = "CreateKeyPair")
+ KeyPairRequest create(@QueryParam("RegionId") String region, @QueryParam("KeyPairName") String keyPairName);
+
+ @Named("sshKeyPair:import")
+ @POST
+ @QueryParams(keys = "Action", values = "ImportKeyPair")
+ KeyPair importKeyPair(@QueryParam("RegionId") String region,
+ @QueryParam("PublicKeyBody") String publicKeyBody,
+ @QueryParam("KeyPairName") String keyPairName);
+
+ @Named("sshKeyPair:delete")
+ @POST
+ @QueryParams(keys = "Action", values = "DeleteKeyPairs")
+ Request delete(@QueryParam("RegionId") String region);
Does this delete all keypairs?
> + @POST
+ @QueryParams(keys = "Action", values = "ImportKeyPair")
+ KeyPair importKeyPair(@QueryParam("RegionId") String region,
+ @QueryParam("PublicKeyBody") String publicKeyBody,
+ @QueryParam("KeyPairName") String keyPairName);
+
+ @Named("sshKeyPair:delete")
+ @POST
+ @QueryParams(keys = "Action", values = "DeleteKeyPairs")
+ Request delete(@QueryParam("RegionId") String region);
+
+ @Named("sshKeyPair:delete")
+ @POST
+ @QueryParams(keys = "Action", values = "DeleteKeyPairs")
+ Request delete(@QueryParam("RegionId") String region, DeleteKeyPairOptions deleteOptions);
+}
Is there a method to get a single keypair?
> + static class ToPagedIterable extends Arg0ToPagedIterable<SecurityGroup, ParseSecurityGroups.ToPagedIterable> {
+
+ private final ECSComputeServiceApi api;
+
+ @Inject
+ ToPagedIterable(ECSComputeServiceApi api) {
+ this.api = api;
+ }
+
+ @Override
+ protected Function<Object, IterableWithMarker<SecurityGroup>> markerToNextForArg0(final Optional<Object> arg0) {
+ return new Function<Object, IterableWithMarker<SecurityGroup>>() {
+ @Override
+ public IterableWithMarker<SecurityGroup> apply(Object input) {
+ String regionId = arg0.get().toString();
+ ListSecurityGroupsOptions listSecurityGroupsOptions = ListSecurityGroupsOptions.Builder.paginationOptions(PaginationOptions.class.cast(input));
You are creating a brand new options object and forgetting all other options in the original object. You need to keep it.
Get a builder out from the input object (so it keeps all info), and just override the pagination options, instead of creating a brand new options object. Apply this to all API classes.
> + return new Function<Object, IterableWithMarker<Tag>>() {
+ @Override
+ public IterableWithMarker<Tag> apply(Object input) {
+ String regionId = arg0.get().toString();
+ ListTagsOptions listTagsOptions = ListTagsOptions.Builder.paginationOptions(PaginationOptions.class.cast(input));
+ return api.tagApi().list(regionId, listTagsOptions);
+ }
+ };
+ }
+ }
+ }
+
+ @Named("tag:add")
+ @POST
+ @QueryParams(keys = "Action", values = "AddTags")
+ @Fallback(Fallbacks.EmptyListOnNotFoundOr404.class)
Remove
> + }
+ }
+
+ @Named("tag:add")
+ @POST
+ @QueryParams(keys = "Action", values = "AddTags")
+ @Fallback(Fallbacks.EmptyListOnNotFoundOr404.class)
+ Request add(@QueryParam("RegionId") String region, @QueryParam("ResourceId") String resourceId,
+ @QueryParam("ResourceType") String resourceType,
+ TagOptions tagOptions);
+
+ @Named("tag:remove")
+ @POST
+ @QueryParams(keys = "Action", values = "RemoveTags")
+ @Fallback(Fallbacks.EmptyListOnNotFoundOr404.class)
+ Request remove(@QueryParam("RegionId") String region,
Does this remove all the tags? Can a single tag be removed?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/441#pullrequestreview-137945744
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] - add more features (#441)
Posted by Andrea Turli <no...@github.com>.
andreaturli commented on this pull request.
> + @Named("sshKeyPair:create")
+ @POST
+ @QueryParams(keys = "Action", values = "CreateKeyPair")
+ KeyPairRequest create(@QueryParam("RegionId") String region, @QueryParam("KeyPairName") String keyPairName);
+
+ @Named("sshKeyPair:import")
+ @POST
+ @QueryParams(keys = "Action", values = "ImportKeyPair")
+ KeyPair importKeyPair(@QueryParam("RegionId") String region,
+ @QueryParam("PublicKeyBody") String publicKeyBody,
+ @QueryParam("KeyPairName") String keyPairName);
+
+ @Named("sshKeyPair:delete")
+ @POST
+ @QueryParams(keys = "Action", values = "DeleteKeyPairs")
+ Request delete(@QueryParam("RegionId") String region);
good point, this is useless. Removing
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/441#discussion_r203271177
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] - add more features (#441)
Posted by Andrea Turli <no...@github.com>.
@andreaturli pushed 1 commit.
f2e09f7 wip
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/441/files/03b707362be9d648ab9bdb89a409837f4a7ac748..f2e09f774d0d5236589bb1d2c075a887f2afaf43
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] - add more features (#441)
Posted by Andrea Turli <no...@github.com>.
@andreaturli pushed 1 commit.
03b7073 address comments
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/441/files/ad4dd9931e0b325f685d274509d99b7123957f46..03b707362be9d648ab9bdb89a409837f4a7ac748
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] - add more features (#441)
Posted by Andrea Turli <no...@github.com>.
squashed and rebased. waiting for the builder to merge it. thanks @nacx
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/441#issuecomment-409311478
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] - add more features (#441)
Posted by Andrea Turli <no...@github.com>.
merged at [master](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/a5dbf006)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/441#issuecomment-409351737
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] - add more features (#441)
Posted by Andrea Turli <no...@github.com>.
andreaturli commented on this pull request.
> + }
+ }
+
+ @Named("tag:add")
+ @POST
+ @QueryParams(keys = "Action", values = "AddTags")
+ @Fallback(Fallbacks.EmptyListOnNotFoundOr404.class)
+ Request add(@QueryParam("RegionId") String region, @QueryParam("ResourceId") String resourceId,
+ @QueryParam("ResourceType") String resourceType,
+ TagOptions tagOptions);
+
+ @Named("tag:remove")
+ @POST
+ @QueryParams(keys = "Action", values = "RemoveTags")
+ @Fallback(Fallbacks.EmptyListOnNotFoundOr404.class)
+ Request remove(@QueryParam("RegionId") String region,
I think so, I'll add a method with optional queryparams to remove one tag only
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/441#discussion_r203304904
Re: [jclouds/jclouds-labs] [JCLOUDS-1430] - add more features (#441)
Posted by Andrea Turli <no...@github.com>.
@nacx I think I've addressed all of your comments except the ones for auto/value as I can't extend a @AutoValue class with a @AutoValue subclass
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/441#issuecomment-405881586