{"action":"created","issue":{"url":"https://api.github.com/repos/servicecatalog/development/issues/349","repository_url":"https://api.github.com/repos/servicecatalog/development","labels_url":"https://api.github.com/repos/servicecatalog/development/issues/349/labels{/name}","comments_url":"https://api.github.com/repos/servicecatalog/development/issues/349/comments","events_url":"https://api.github.com/repos/servicecatalog/development/issues/349/events","html_url":"https://github.com/servicecatalog/development/pull/349","id":181085832,"number":349,"title":" Added Public IP, Subnet and Security group functionality in AWS controller (oscm-app-aws)","user":{"login":"pujarisat","id":22190140,"avatar_url":"https://avatars.githubusercontent.com/u/22190140?v=3","gravatar_id":"","url":"https://api.github.com/users/pujarisat","html_url":"https://github.com/pujarisat","followers_url":"https://api.github.com/users/pujarisat/followers","following_url":"https://api.github.com/users/pujarisat/following{/other_user}","gists_url":"https://api.github.com/users/pujarisat/gists{/gist_id}","starred_url":"https://api.github.com/users/pujarisat/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pujarisat/subscriptions","organizations_url":"https://api.github.com/users/pujarisat/orgs","repos_url":"https://api.github.com/users/pujarisat/repos","events_url":"https://api.github.com/users/pujarisat/events{/privacy}","received_events_url":"https://api.github.com/users/pujarisat/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2016-10-05T07:48:32Z","updated_at":"2016-10-06T14:15:38Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/servicecatalog/development/pulls/349","html_url":"https://github.com/servicecatalog/development/pull/349","diff_url":"https://github.com/servicecatalog/development/pull/349.diff","patch_url":"https://github.com/servicecatalog/development/pull/349.patch"},"body":"Hi Team,\r\n\r\nWe have added below functionality on AWS controller and also we have modified the unit test classes as per the new functionality \r\n\r\n1)\tApply the public IP to the AWS instance\r\n2)\tAdd the subnet to the AWS instance(Select the existing subnet and apply same to AWS instance)\r\n3)\tAdd security group to the AWS instance \r\n4)\tAdd disk size to AWS instance\r\n\r\n-Thanks\r\nSatling\n\n<!-- Reviewable:start -->\n---\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/servicecatalog/development/349)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/servicecatalog/development/issues/comments/251974837","html_url":"https://github.com/servicecatalog/development/pull/349#issuecomment-251974837","issue_url":"https://api.github.com/repos/servicecatalog/development/issues/349","id":251974837,"user":{"login":"StavrevaS","id":14818282,"avatar_url":"https://avatars.githubusercontent.com/u/14818282?v=3","gravatar_id":"","url":"https://api.github.com/users/StavrevaS","html_url":"https://github.com/StavrevaS","followers_url":"https://api.github.com/users/StavrevaS/followers","following_url":"https://api.github.com/users/StavrevaS/following{/other_user}","gists_url":"https://api.github.com/users/StavrevaS/gists{/gist_id}","starred_url":"https://api.github.com/users/StavrevaS/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/StavrevaS/subscriptions","organizations_url":"https://api.github.com/users/StavrevaS/orgs","repos_url":"https://api.github.com/users/StavrevaS/repos","events_url":"https://api.github.com/users/StavrevaS/events{/privacy}","received_events_url":"https://api.github.com/users/StavrevaS/received_events","type":"User","site_admin":false},"created_at":"2016-10-06T14:15:37Z","updated_at":"2016-10-06T14:15:37Z","body":"\n\n\n\nReviewed 7 of 7 files at r3.\nReview status: 30 of 32 files reviewed at latest revision, 19 unresolved discussions, some commit checks failed.\n\n---\n\n*[oscm-app-aws/.classpath, line 24 at r2](https://reviewable.io:443/reviews/servicecatalog/development/349#-KTJRvEuCqfnEMniBhiE:-KTPGdd5TtlLQTl8GsSb:b32w0x) ([raw file](https://github.com/servicecatalog/development/blob/a94face0a470a8b4d16fa83fd2969f4be6fb2234/oscm-app-aws/.classpath#L24)):*\n<details><summary><i>Previously, StavrevaS (Suzana Stavreva) wrote…</i></summary>\n> Return back to the old dependencies where possible (see comment by ivy-build.xml)\n</details>\nOK\n\n---\n\n*[oscm-app-aws/package.properties, line 19 at r2](https://reviewable.io:443/reviews/servicecatalog/development/349#-KTJS9Sy3rH6SbLoUDyJ:-KTPGiojx9pCBuHEG8BB:b32w0x) ([raw file](https://github.com/servicecatalog/development/blob/a94face0a470a8b4d16fa83fd2969f4be6fb2234/oscm-app-aws/package.properties#L19)):*\n<details><summary><i>Previously, StavrevaS (Suzana Stavreva) wrote…</i></summary>\n> Consider comment for ivy-build.xml\n</details>\nOK\n\n---\n\n*[oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java, line 70 at r2](https://reviewable.io:443/reviews/servicecatalog/development/349#-KTJoyW-oUWF_CfnW64g:-KTPGw2ZpgG7kLEPxnC7:b32w0x) ([raw file](https://github.com/servicecatalog/development/blob/01cb08c39ae795207e6e6d5696aaa9a6ed448387/oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java#L70)):*\n<details><summary><i>Previously, StavrevaS (Suzana Stavreva) wrote…</i></summary>\n> Why the user and password are removed?\n</details>\nOK\n\n---\n\n*[oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java, line 230 at r2](https://reviewable.io:443/reviews/servicecatalog/development/349#-KTJpFzGVhuaL5c-dpLx:-KTPH3r7KVl_oTKVF0sk:b-tu6uf1) ([raw file](https://github.com/servicecatalog/development/blob/a94face0a470a8b4d16fa83fd2969f4be6fb2234/oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java#L230)):*\n<details><summary><i>Previously, StavrevaS (Suzana Stavreva) wrote…</i></summary>\n> Remove println, move info to debug statement.\n</details>\nDescribe in the debug statement the values (e.g \"Image ID: \"+image.getImageId() etc)\n\n---\n\n*[oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java, line 235 at r2](https://reviewable.io:443/reviews/servicecatalog/development/349#-KTJpPX2896jkR9u12XI:-KTPHLk7IO6dHwSdXeZm:b32w0x) ([raw file](https://github.com/servicecatalog/development/blob/a94face0a470a8b4d16fa83fd2969f4be6fb2234/oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java#L235)):*\n<details><summary><i>Previously, StavrevaS (Suzana Stavreva) wrote…</i></summary>\n> Use Messages.getAll() like in the original code, why this is removed? This error massage may be used from APP for mail notification and should be localized.\n</details>\nOK\n\n---\n\n*[oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java, line 259 at r2](https://reviewable.io:443/reviews/servicecatalog/development/349#-KTJqJkuIKCxyzna8ggu:-KTPHQTOdrMKbTW_1mv7:b32w0x) ([raw file](https://github.com/servicecatalog/development/blob/a94face0a470a8b4d16fa83fd2969f4be6fb2234/oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java#L259)):*\n<details><summary><i>Previously, StavrevaS (Suzana Stavreva) wrote…</i></summary>\n> Define localized error message, like in resolveAMI() method.\n</details>\nOK\n\n---\n\n*[oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java, line 267 at r2](https://reviewable.io:443/reviews/servicecatalog/development/349#-KTJrGCAlsFpCrpAI7lK:-KTPHcCYyutOzWR-U0fJ:b32w0x) ([raw file](https://github.com/servicecatalog/development/blob/a94face0a470a8b4d16fa83fd2969f4be6fb2234/oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java#L267)):*\n<details><summary><i>Previously, StavrevaS (Suzana Stavreva) wrote…</i></summary>\n> Describe param vpcId.\n</details>\nOK\n\n---\n\n*[oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java, line 305 at r2](https://reviewable.io:443/reviews/servicecatalog/development/349#-KTJqgJQQQywDvtNMGuG:-KTPHgHIZ-u1Wjhfz4VP:b32w0x) ([raw file](https://github.com/servicecatalog/development/blob/a94face0a470a8b4d16fa83fd2969f4be6fb2234/oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java#L305)):*\n<details><summary><i>Previously, StavrevaS (Suzana Stavreva) wrote…</i></summary>\n> Define localized error message like in removeAMI() method.\n</details>\nOK\n\n---\n\n*[oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java, line 338 at r2](https://reviewable.io:443/reviews/servicecatalog/development/349#-KTJs7loRibTvJ_Ej9wI:-KTPHi42CAxEjxSV_IPg:b32w0x) ([raw file](https://github.com/servicecatalog/development/blob/a94face0a470a8b4d16fa83fd2969f4be6fb2234/oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java#L338)):*\n<details><summary><i>Previously, StavrevaS (Suzana Stavreva) wrote…</i></summary>\n> Do not log keypair name, even for debug purposes.\n</details>\nOK\n\n---\n\n*[oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java, line 350 at r2](https://reviewable.io:443/reviews/servicecatalog/development/349#-KTJsqY3aFpixpIwcRQZ:-KTPHs06F-71mnomOxJ7:b32w0x) ([raw file](https://github.com/servicecatalog/development/blob/a94face0a470a8b4d16fa83fd2969f4be6fb2234/oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java#L350)):*\n<details><summary><i>Previously, StavrevaS (Suzana Stavreva) wrote…</i></summary>\n> > Boolean.parseBoolean(ph.getPublicIp()))\n> \n> It would be good to trim the string to be parsed if it is not null.\n> \n</details>\nOK\n\n---\n\n*[oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java, line 385 at r2](https://reviewable.io:443/reviews/servicecatalog/development/349#-KTJv8zpDoMc8LlHKiVI:-KTPHuXezPNn7m6Hxacp:b32w0x) ([raw file](https://github.com/servicecatalog/development/blob/a94face0a470a8b4d16fa83fd2969f4be6fb2234/oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java#L385)):*\n<details><summary><i>Previously, StavrevaS (Suzana Stavreva) wrote…</i></summary>\n> Too many code duplication in the if statement. Please refactor this.\n</details>\nOK\n\n---\n\n*[oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java, line 417 at r2](https://reviewable.io:443/reviews/servicecatalog/development/349#-KTJvaMOZ6sji6e1nonA:-KTPIWCiGOH9wOsIIvzB:b32w0x) ([raw file](https://github.com/servicecatalog/development/blob/a94face0a470a8b4d16fa83fd2969f4be6fb2234/oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java#L417)):*\n<details><summary><i>Previously, StavrevaS (Suzana Stavreva) wrote…</i></summary>\n> Please convert this to proper APPlatformException, with localized error text\n</details>\nOK\n\n---\n\n*[oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java, line 569 at r2](https://reviewable.io:443/reviews/servicecatalog/development/349#-KTJvzN49VkKWIkIJw48:-KTPI4oFKa-fMPEC17_h:b32w0x) ([raw file](https://github.com/servicecatalog/development/blob/a94face0a470a8b4d16fa83fd2969f4be6fb2234/oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java#L569)):*\n<details><summary><i>Previously, StavrevaS (Suzana Stavreva) wrote…</i></summary>\n> Why these tags are removed?\n</details>\nOK\n\n---\n\n*[oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java, line 274 at r3](https://reviewable.io:443/reviews/servicecatalog/development/349#-KTPHSYLtwsNKYoHgn1i:-KTPHSYM-2WTWYiIOXTQ:bm98ehf) ([raw file](https://github.com/servicecatalog/development/blob/ece42bba3a13c0db67a76f7597eb2d24a9bdec7e/oscm-app-aws/javasrc/org/oscm/app/aws/EC2Communication.java#L274)):*\n> ```Java\n>         throw new APPlatformException(\n>                 Messages.getAll(\"error_invalid_subnet_id \") + subnetString);\n> ```\n\nWhite space before closing \"?\n\n---\n\n*[oscm-app-aws/javasrc/org/oscm/app/aws/controller/AWSController.java, line 496 at r2](https://reviewable.io:443/reviews/servicecatalog/development/349#-KTJwGTv5bw-fqvJKXEo:-KTPIck1Rro0F6fu-xnC:b32w0x) ([raw file](https://github.com/servicecatalog/development/blob/a94face0a470a8b4d16fa83fd2969f4be6fb2234/oscm-app-aws/javasrc/org/oscm/app/aws/controller/AWSController.java#L496)):*\n<details><summary><i>Previously, StavrevaS (Suzana Stavreva) wrote…</i></summary>\n> PLease check if this code about disk size and subnet is necesssary here. The method deals with operations like start/stop instance, not provisioning actions like create instance.\n</details>\nOK\n\n---\n\n*[oscm-app-aws/javasrc/org/oscm/app/aws/controller/EC2Processor.java, line 231 at r2](https://reviewable.io:443/reviews/servicecatalog/development/349#-KTJw_y2HEb8POnXMgEY:-KTPIgZ6-Ey94Ezl02mo:b32w0x) ([raw file](https://github.com/servicecatalog/development/blob/a94face0a470a8b4d16fa83fd2969f4be6fb2234/oscm-app-aws/javasrc/org/oscm/app/aws/controller/EC2Processor.java#L231)):*\n<details><summary><i>Previously, StavrevaS (Suzana Stavreva) wrote…</i></summary>\n> Check spelling. Make debug statement.\n</details>\nOK\n\n---\n\n*[oscm-app-aws/resources/TechnicalService_AWS.xml, line 83 at r2](https://reviewable.io:443/reviews/servicecatalog/development/349#-KTJPJAn41JTMpnzQY1W:-KTPIkw0ESJp6Hy0rMYh:b32w0x) ([raw file](https://github.com/servicecatalog/development/blob/a94face0a470a8b4d16fa83fd2969f4be6fb2234/oscm-app-aws/resources/TechnicalService_AWS.xml#L83)):*\n<details><summary><i>Previously, StavrevaS (Suzana Stavreva) wrote…</i></summary>\n> Can you explain, why these parameters should be mandatory?\n</details>\nOK\n\n---\n\n*[oscm-build/ivy-build.xml, line 103 at r2](https://reviewable.io:443/reviews/servicecatalog/development/349#-KTJPWlSrt-hzHLDp_vt:-KTPIpPF2C8KE9GBpvY6:b32w0x) ([raw file](https://github.com/servicecatalog/development/blob/a94face0a470a8b4d16fa83fd2969f4be6fb2234/oscm-build/ivy-build.xml#L103)):*\n<details><summary><i>Previously, StavrevaS (Suzana Stavreva) wrote…</i></summary>\n> Most of the libraries are already included in the right or older version. Please update where necessary and adapt packaging where necessary.\n> \n> Jackson libraries:\n> already included in this file in the requested version in /libraries/jackson/javalib/. They should not be duplicated for aws. \n> Http libraries:\n> already included in /libraries/apache-httpcore/javalib in different versions from requested. Please update here.\n> Joda time library:\n> Include in new dir /libraries/joda-time/javalib\n</details>\nOK\n\n---\n\n*[oscm-build/ivy.xml, line 525 at r2](https://reviewable.io:443/reviews/servicecatalog/development/349#-KTJRWDSfu7GrmmbFvX5:-KTPLE81CXRLtWhntXa4:b32w0x) ([raw file](https://github.com/servicecatalog/development/blob/a94face0a470a8b4d16fa83fd2969f4be6fb2234/oscm-build/ivy.xml#L525)):*\n<details><summary><i>Previously, StavrevaS (Suzana Stavreva) wrote…</i></summary>\n> Adapt if ivy-build.xml is changed.\n</details>\nOK\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/servicecatalog/development/349)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"StavrevaS"}}