You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stratos.apache.org by pubudu538 <gi...@git.apache.org> on 2015/06/29 09:02:08 UTC

[GitHub] stratos pull request: Fixed not showing exceptions when adding an ...

GitHub user pubudu538 opened a pull request:

    https://github.com/apache/stratos/pull/380

    Fixed not showing exceptions when adding an application

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/pubudu538/stratos add-app

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/stratos/pull/380.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 #380
    
----
commit e3bae228c12b6222a560bc178c88ed08e96cd9d0
Author: Pubudu Gunatilaka <pu...@gmail.com>
Date:   2015-06-29T07:00:49Z

    fixed not showing exceptions in adding an application

----


---
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: Fixed not showing exceptions when adding an ...

Posted by imesh <gi...@git.apache.org>.
Github user imesh commented on a diff in the pull request:

    https://github.com/apache/stratos/pull/380#discussion_r46095170
  
    --- Diff: components/org.apache.stratos.rest.endpoint/src/main/java/org/apache/stratos/rest/endpoint/api/StratosApiV41Utils.java ---
    @@ -1396,7 +1396,8 @@ public static void addApplication(ApplicationBean appDefinition, ConfigurationCo
                         usedCartridgeGroups.toArray(new String[usedCartridgeGroups.size()]));
     
             } catch (AutoscalerServiceApplicationDefinitionExceptionException e) {
    -            throw new RestAPIException(e);
    +            String message = e.getFaultMessage().getApplicationDefinitionException().getMessage();
    --- End diff --
    
    I think we may not need to handle null here, because if a NPE is raised it means that an error message raised by the Autoscaler service is not sent back to the client properly. If so we need to fix that problem. If we add a null check it would return a blank error message to the client.


---
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: Fixed not showing exceptions when adding an ...

Posted by malakasilva <gi...@git.apache.org>.
Github user malakasilva commented on a diff in the pull request:

    https://github.com/apache/stratos/pull/380#discussion_r46079507
  
    --- Diff: components/org.apache.stratos.rest.endpoint/src/main/java/org/apache/stratos/rest/endpoint/api/StratosApiV41Utils.java ---
    @@ -1396,7 +1396,8 @@ public static void addApplication(ApplicationBean appDefinition, ConfigurationCo
                         usedCartridgeGroups.toArray(new String[usedCartridgeGroups.size()]));
     
             } catch (AutoscalerServiceApplicationDefinitionExceptionException e) {
    -            throw new RestAPIException(e);
    +            String message = e.getFaultMessage().getApplicationDefinitionException().getMessage();
    --- End diff --
    
    Can this lead to nullpoint exceptions?



---
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: Fixed not showing exceptions when adding an ...

Posted by pubudu538 <gi...@git.apache.org>.
Github user pubudu538 commented on a diff in the pull request:

    https://github.com/apache/stratos/pull/380#discussion_r46094208
  
    --- Diff: components/org.apache.stratos.rest.endpoint/src/main/java/org/apache/stratos/rest/endpoint/api/StratosApiV41Utils.java ---
    @@ -1396,7 +1396,8 @@ public static void addApplication(ApplicationBean appDefinition, ConfigurationCo
                         usedCartridgeGroups.toArray(new String[usedCartridgeGroups.size()]));
     
             } catch (AutoscalerServiceApplicationDefinitionExceptionException e) {
    -            throw new RestAPIException(e);
    +            String message = e.getFaultMessage().getApplicationDefinitionException().getMessage();
    --- End diff --
    
    I will add the null check there and include a proper error message for the end user.


---
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: Fixed not showing exceptions when adding an ...

Posted by imesh <gi...@git.apache.org>.
Github user imesh commented on a diff in the pull request:

    https://github.com/apache/stratos/pull/380#discussion_r46093927
  
    --- Diff: components/org.apache.stratos.rest.endpoint/src/main/java/org/apache/stratos/rest/endpoint/api/StratosApiV41Utils.java ---
    @@ -1396,7 +1396,8 @@ public static void addApplication(ApplicationBean appDefinition, ConfigurationCo
                         usedCartridgeGroups.toArray(new String[usedCartridgeGroups.size()]));
     
             } catch (AutoscalerServiceApplicationDefinitionExceptionException e) {
    -            throw new RestAPIException(e);
    +            String message = e.getFaultMessage().getApplicationDefinitionException().getMessage();
    --- End diff --
    
    Thanks for pointing Malaka! Ideally this should not raise any NPEs because autoscaler service always returns an application definition exception and an error message. However it might be better to add null checks here. @pubudu538 please note!


---
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: Fixed not showing exceptions when adding an ...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/stratos/pull/380


---
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.
---