You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stratos.apache.org by Vishanth <gi...@git.apache.org> on 2015/03/06 13:25:53 UTC
[GitHub] stratos pull request: Added exception for persisting autoscaler po...
GitHub user Vishanth opened a pull request:
https://github.com/apache/stratos/pull/261
Added exception for persisting autoscaler policy in registry manager
Added registry exception for persisting autoscaler policy in registry manager
Also ensured the autoscaler policy is not added to the information model in policy manager, if the adding of the autoscaler policy is unsuccessful
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Vishanth/stratos master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/stratos/pull/261.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #261
----
commit a3f9bc4572762440d5cd20c8b40455c49aadce09
Author: Vishanth <vi...@gmail.com>
Date: 2015-03-06T12:12:03Z
Adding exception for persisting autoscaler policy
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] stratos pull request: Added exception for persisting autoscaler po...
Posted by Vishanth <gi...@git.apache.org>.
Github user Vishanth commented on a diff in the pull request:
https://github.com/apache/stratos/pull/261#discussion_r27942517
--- Diff: components/org.apache.stratos.autoscaler/src/main/java/org/apache/stratos/autoscaler/pojo/policy/PolicyManager.java ---
@@ -71,11 +71,11 @@ public boolean addAutoscalePolicy(AutoscalePolicy policy) throws InvalidPolicyEx
if (log.isInfoEnabled()) {
log.info(String.format("Adding autoscaling policy: [id] %s", policy.getId()));
}
- if(StringUtils.isEmpty(policy.getId())){
+ if (StringUtils.isEmpty(policy.getId())) {
throw new AutoScalerException("Autoscaling policy id cannot be empty");
}
- this.addASPolicyToInformationModel(policy);
RegistryManager.getInstance().persistAutoscalerPolicy(policy);
+ this.addASPolicyToInformationModel(policy);
if (log.isInfoEnabled()) {
log.info(String.format("Autoscaling policy is added successfully: [id] %s", policy.getId()));
--- End diff --
For consistency we have been using the string format in all our loggings. But true it is not much effective for a single string.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] stratos pull request: Added exception for persisting autoscaler po...
Posted by prabathabey <gi...@git.apache.org>.
Github user prabathabey commented on a diff in the pull request:
https://github.com/apache/stratos/pull/261#discussion_r27852400
--- Diff: components/org.apache.stratos.autoscaler/src/main/java/org/apache/stratos/autoscaler/registry/RegistryManager.java ---
@@ -117,11 +117,17 @@ private void persist(Object dataObj, String resourcePath) throws AutoScalerExcep
}
public void persistAutoscalerPolicy(AutoscalePolicy autoscalePolicy) {
- String resourcePath = AutoscalerConstants.AUTOSCALER_RESOURCE + AutoscalerConstants.AS_POLICY_RESOURCE + "/" + autoscalePolicy.getId();
- persist(autoscalePolicy, resourcePath);
- if (log.isDebugEnabled()) {
- log.debug(String.format("Autoscaler policy written to registry: [id] %s [name] %s [description] %s",
- autoscalePolicy.getId(), autoscalePolicy.getDisplayName(), autoscalePolicy.getDescription()));
+ try {
+ String resourcePath = AutoscalerConstants.AUTOSCALER_RESOURCE +
+ AutoscalerConstants.AS_POLICY_RESOURCE + "/" + autoscalePolicy.getId();
+ persist(autoscalePolicy, resourcePath);
+ if (log.isDebugEnabled()) {
+ log.debug(String.format("Autoscaler policy written to registry: [id] %s [name] %s [description] %s",
+ autoscalePolicy.getId(), autoscalePolicy.getDisplayName(), autoscalePolicy.getDescription()));
+ }
+ } catch (Exception e) {
--- End diff --
If catching generic exceptions was done intentionally, maybe it's good to add a comment explaining why it was done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] stratos pull request: Added exception for persisting autoscaler po...
Posted by muthulee <gi...@git.apache.org>.
Github user muthulee commented on a diff in the pull request:
https://github.com/apache/stratos/pull/261#discussion_r27940548
--- Diff: components/org.apache.stratos.autoscaler/src/main/java/org/apache/stratos/autoscaler/pojo/policy/PolicyManager.java ---
@@ -71,11 +71,11 @@ public boolean addAutoscalePolicy(AutoscalePolicy policy) throws InvalidPolicyEx
if (log.isInfoEnabled()) {
log.info(String.format("Adding autoscaling policy: [id] %s", policy.getId()));
}
- if(StringUtils.isEmpty(policy.getId())){
+ if (StringUtils.isEmpty(policy.getId())) {
throw new AutoScalerException("Autoscaling policy id cannot be empty");
--- End diff --
More informative logs. What policy is empty?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] stratos pull request: Added exception for persisting autoscaler po...
Posted by Vishanth <gi...@git.apache.org>.
Github user Vishanth commented on a diff in the pull request:
https://github.com/apache/stratos/pull/261#discussion_r27942541
--- Diff: components/org.apache.stratos.autoscaler/src/main/java/org/apache/stratos/autoscaler/registry/RegistryManager.java ---
@@ -117,11 +117,17 @@ private void persist(Object dataObj, String resourcePath) throws AutoScalerExcep
}
public void persistAutoscalerPolicy(AutoscalePolicy autoscalePolicy) {
- String resourcePath = AutoscalerConstants.AUTOSCALER_RESOURCE + AutoscalerConstants.AS_POLICY_RESOURCE + "/" + autoscalePolicy.getId();
- persist(autoscalePolicy, resourcePath);
- if (log.isDebugEnabled()) {
- log.debug(String.format("Autoscaler policy written to registry: [id] %s [name] %s [description] %s",
- autoscalePolicy.getId(), autoscalePolicy.getDisplayName(), autoscalePolicy.getDescription()));
+ try {
+ String resourcePath = AutoscalerConstants.AUTOSCALER_RESOURCE +
+ AutoscalerConstants.AS_POLICY_RESOURCE + "/" + autoscalePolicy.getId();
+ persist(autoscalePolicy, resourcePath);
+ if (log.isDebugEnabled()) {
+ log.debug(String.format("Autoscaler policy written to registry: [id] %s [name] %s [description] %s",
+ autoscalePolicy.getId(), autoscalePolicy.getDisplayName(), autoscalePolicy.getDescription()));
+ }
+ } catch (Exception e) {
--- End diff --
Noted. Will do.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] stratos pull request: Added exception for persisting autoscaler po...
Posted by muthulee <gi...@git.apache.org>.
Github user muthulee commented on a diff in the pull request:
https://github.com/apache/stratos/pull/261#discussion_r27940566
--- Diff: components/org.apache.stratos.autoscaler/src/main/java/org/apache/stratos/autoscaler/pojo/policy/PolicyManager.java ---
@@ -71,11 +71,11 @@ public boolean addAutoscalePolicy(AutoscalePolicy policy) throws InvalidPolicyEx
if (log.isInfoEnabled()) {
log.info(String.format("Adding autoscaling policy: [id] %s", policy.getId()));
}
- if(StringUtils.isEmpty(policy.getId())){
+ if (StringUtils.isEmpty(policy.getId())) {
throw new AutoScalerException("Autoscaling policy id cannot be empty");
}
- this.addASPolicyToInformationModel(policy);
RegistryManager.getInstance().persistAutoscalerPolicy(policy);
+ this.addASPolicyToInformationModel(policy);
if (log.isInfoEnabled()) {
log.info(String.format("Autoscaling policy is added successfully: [id] %s", policy.getId()));
--- End diff --
Why use format for one string?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] stratos pull request: Added exception for persisting autoscaler po...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/stratos/pull/261
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] stratos pull request: Added exception for persisting autoscaler po...
Posted by imesh <gi...@git.apache.org>.
Github user imesh commented on the pull request:
https://github.com/apache/stratos/pull/261#issuecomment-77554002
Thanks for the fix @Vishanth! I have now merged this to master branch.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---