Dear Hong Zhang:
>The changes look good. Please run QuickLook tests and deployment dev
>tests to make sure the changes do not introduce any regressions (for
>this set of changes, deployment dev tests in PE mode should be sufficient).
>Intructions to deployment dev tests:
>https://wikis.oracle.com/display/GlassFish/4.0DeploymentDevTests
>Thanks,
Thanks for your suggestion, I'll run QuickLook tests and upload the test
results to my github.
--Best Regards
--Jeremy Lv
-----Original Message-----
From: Hong Zhang [mailto:hong.hz.zhang_at_oracle.com]
Sent: Wednesday, July 25, 2012 11:06 PM
To: dev_at_glassfish.java.net
Subject: Re: About GLASSFISH-18916
The changes look good. Please run QuickLook tests and deployment dev
tests to make sure the changes do not introduce any regressions (for
this set of changes, deployment dev tests in PE mode should be sufficient).
Intructions to deployment dev tests:
https://wikis.oracle.com/display/GlassFish/4.0DeploymentDevTests
Thanks,
- Hong
On 7/25/2012 5:50 AM, Lv Songping wrote:
> Dear Hong Zhang:
>
>> Thanks for making the changes. Please see my comments below:
>> 1. The modified files are in deployment/admin module and not in
>> core/kernel.
>> 2. For DeployCommand:
>> a. As we are already throwing the exception in the if block, we don't
>> need to put the rest of the code in the else block here.
>> b. For error message, let's do something else
>> lifecyclemodule_withsamename_exists=Lifecycle module with same name {0}
>> already exists, please pick a different name for the application.
>> 3. For CreateLifecycleModuleCommand:
>> a. For the import statement, just import the new class you are using
>> (Applications) and not import everything
>> b. Please don't change the REST endpoints as this is not the deploy
>> command.
>> c. For the checks you made in validateTarget, you can just add the check
>> in the beginning of this method or even outside of the method (you don't
>> really need to go into each block of this method and do similar
>> validation as this check is not relevant to target anyways, the name has
>> to be unique per domain).
>> Application app = apps.getApplication(name);
>> if(app != null && !app.isLifecycleModule()){
>> // throw illegal exception here
>> }
>> with error string like this:
>> application_withsamename_exists=Application with same name {0} already
>> exists, please pick a different name for the lifecycle module.
> Thanks for your advices; I have updated my github under your suggestions.
> Please check it again.
>
> --Best Regards
> --Jeremy Lv
> -----Original Message-----
> From: Hong Zhang [mailto:hong.hz.zhang_at_oracle.com]
> Sent: Tuesday, July 24, 2012 3:00 AM
> To: dev_at_glassfish.java.net
> Subject: Re: About GLASSFISH-18916
>
> Hi, Jeremy
> Thanks for making the changes. Please see my comments below:
> 1. The modified files are in deployment/admin module and not in
> core/kernel.
> 2. For DeployCommand:
> a. As we are already throwing the exception in the if block, we don't
> need to put the rest of the code in the else block here.
> b. For error message, let's do something else
> lifecyclemodule_withsamename_exists=Lifecycle module with same name {0}
> already exists, please pick a different name for the application.
> 3. For CreateLifecycleModuleCommand:
> a. For the import statement, just import the new class you are using
> (Applications) and not import everything
> b. Please don't change the REST endpoints as this is not the deploy
> command.
> c. For the checks you made in validateTarget, you can just add the check
> in the beginning of this method or even outside of the method (you don't
> really need to go into each block of this method and do similar
> validation as this check is not relevant to target anyways, the name has
> to be unique per domain).
> Application app = apps.getApplication(name);
> if(app != null && !app.isLifecycleModule()){
> // throw illegal exception here
> }
> with error string like this:
> application_withsamename_exists=Application with same name {0} already
> exists, please pick a different name for the lifecycle module.
>
> - Hong
>
> On 7/23/2012 4:27 AM, Lv Songping wrote:
>> Dear Hong Zhang:
>>
>> I have revised the issue(GLASSFISH-18916) about the " Create a
>> LifecycleModule, then deploy a application Module that has the same name
>> with the LifecycleModule, NullPointerException happened " under your
> advices
>> and reflected the modified files into
>> https://github.com/LvSongping/GLASSFISH-18916
>>
>> please review it again.
>>
>>
>> --best regards
>> --Jeremy Lv
>>
>> ÒÔÉÏ
>> ÂÀËÎƽ
>> ----------------------------------------------------
>> Lv Songping
>> Development Dept.II
>> No. 6 Wenzhu Road, Nanjing, 210012, China
>> TEL: +86+25-86630566-8196
>> COINS: 7998-8196
>> FAX: +86+25-83317685
>> Mail: lvsongping_at_cn.fujitsu.com
>> ----------------------------------------------------
>> This communication is for use by the intended recipient(s) only and may
>> contain information that is privileged, confidential and exempt from
>> disclosure under applicable law. If you are not an intended recipient of
>> this communication, you are hereby notified that any dissemination,
>> distribution or copying hereof is strictly prohibited. If you have
> received
>> this communication in error, please notify me by reply e-mail,
> permanently
>> delete this communication from your system, and destroy any hard copies
> you
>> may have printed.
>>
>> -----Original Message-----
>> From: Hong Zhang [mailto:hong.hz.zhang_at_oracle.com]
>> Sent: Thursday, July 19, 2012 10:56 PM
>> To: dev_at_glassfish.java.net
>> Subject: Re: About GLASSFISH-18916
>>
>> Hi Jeremy
>> Thanks for looking into this issue. I think we need to do some
>> thinking first on this issue on what is the desirable behavior here.
>> Normally when you redeploy an application, it undeploys the old
>> version of the application, and deploys the new version of the
>> application. But here the old version of the application and the new
>> version of the application are very different, one is a lifecycle module
>> and the other is a regular Java EE application, and the code paths for
>> deploy and undeploy lifecycle module and Java EE application are quite
>> different also. Two options here, one is to delete the lifecycle module
>> as part of the undeployment, and then deploy the new application. The
>> other option is to disallow this type of redeployment, and print a
>> meaningful error message to user.
>> I prefer the second option as it is the simpler approach (without
>> needing to write a lot of code), and should be sufficient to handle this
>> not too common use case. The checks would need to be made from both
>> sides for handling the two cases (create a lifecycle module first, then
>> deploy an application with same name, or deploy an application first,
>> and then create a lifecycle module with same name), so you would need to
>> change both CreateLifecycleModuleCommand and DeployCommand. For
>> DeployCommand, the check could be made in the handleRedeploy method
>> after we get the current app. And in CreateLifecyleModuleCommand, the
>> check could be made inside the validateTarget method.
>>
>> Thanks,
>>
>> - Hong
>>
>> On 7/18/2012 10:28 PM, Lv Songping wrote:
>>> Dear Hong Zhang
>>> Cc:Glassfish deployment team
>>>
>>> I have revised the issue(GLASSFISH-18916) about the " Create a
>>> LifecycleModule, then deploy a application Module that has the same name
>>> with the LifecycleModule, NullPointerException happened " and reflected
>> the
>>> modified files into https://github.com/LvSongping/GLASSFISH-18916 and
>>> please review it and give me some advices.
>>>
>>> The NPE is occurred because of the LifecycleModule don't have the
>> attribute
>>> named "Location", when execute the " uri = new
>>> URI(application.getLocation())" in UndeployCommand.java, it will throw
> out
>>> the NPE.
>>>
>>> The issue url is as follows:
>>> http://java.net/jira/browse/GLASSFISH-18916
>>>
>>> --Best Regard!
>>> --Jeremy Lv
>>>
>>>
>