You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by Jaimin Jetly <ja...@hortonworks.com> on 2014/11/01 19:48:33 UTC

Review Request 27471: Install Wizard completely breaks after install fails and going back to a previous step

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27471/
-----------------------------------------------------------

Review request for Ambari, Srimanth Gunturi and Yusaku Sako.


Bugs: AMBARI-8094
    https://issues.apache.org/jira/browse/AMBARI-8094


Repository: ambari


Description
-------

STR:
Attempted to deploy a cluster.
Install failed.
Went back to a previous step using the left nav.
It gave me the pop-up warning that I would lose work to date, I said ok, and it did nothing. The popup went away and the display remained on the Install Results page.


Diffs
-----

  ambari-web/app/router.js 97fc560 
  ambari-web/app/routes/installer.js 5c164fe 

Diff: https://reviews.apache.org/r/27471/diff/


Testing
-------

tested e2e on the cluster that had the repro for this issue


Thanks,

Jaimin Jetly


Re: Review Request 27471: Install Wizard completely breaks after install fails and going back to a previous step

Posted by Yusaku Sako <yu...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27471/#review59507
-----------------------------------------------------------

Ship it!


Ship It!

- Yusaku Sako


On Nov. 1, 2014, 10:20 p.m., Jaimin Jetly wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27471/
> -----------------------------------------------------------
> 
> (Updated Nov. 1, 2014, 10:20 p.m.)
> 
> 
> Review request for Ambari, Srimanth Gunturi and Yusaku Sako.
> 
> 
> Bugs: AMBARI-8094
>     https://issues.apache.org/jira/browse/AMBARI-8094
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> STR:
> Attempted to deploy a cluster.
> Install failed.
> Went back to a previous step using the left nav.
> It gave me the pop-up warning that I would lose work to date, I said ok, and it did nothing. The popup went away and the display remained on the Install Results page.
> 
> 
> Diffs
> -----
> 
>   ambari-web/app/router.js 97fc560 
>   ambari-web/app/routes/installer.js 5c164fe 
> 
> Diff: https://reviews.apache.org/r/27471/diff/
> 
> 
> Testing
> -------
> 
> tested e2e on the cluster that had the repro for this issue
> 
> 
> Thanks,
> 
> Jaimin Jetly
> 
>


Re: Review Request 27471: Install Wizard completely breaks after install fails and going back to a previous step

Posted by Srimanth Gunturi <sr...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27471/#review59508
-----------------------------------------------------------

Ship it!


Ship It!

- Srimanth Gunturi


On Nov. 1, 2014, 10:20 p.m., Jaimin Jetly wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27471/
> -----------------------------------------------------------
> 
> (Updated Nov. 1, 2014, 10:20 p.m.)
> 
> 
> Review request for Ambari, Srimanth Gunturi and Yusaku Sako.
> 
> 
> Bugs: AMBARI-8094
>     https://issues.apache.org/jira/browse/AMBARI-8094
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> STR:
> Attempted to deploy a cluster.
> Install failed.
> Went back to a previous step using the left nav.
> It gave me the pop-up warning that I would lose work to date, I said ok, and it did nothing. The popup went away and the display remained on the Install Results page.
> 
> 
> Diffs
> -----
> 
>   ambari-web/app/router.js 97fc560 
>   ambari-web/app/routes/installer.js 5c164fe 
> 
> Diff: https://reviews.apache.org/r/27471/diff/
> 
> 
> Testing
> -------
> 
> tested e2e on the cluster that had the repro for this issue
> 
> 
> Thanks,
> 
> Jaimin Jetly
> 
>


Re: Review Request 27471: Install Wizard completely breaks after install fails and going back to a previous step

Posted by Jaimin Jetly <ja...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27471/
-----------------------------------------------------------

(Updated Nov. 1, 2014, 10:20 p.m.)


Review request for Ambari, Srimanth Gunturi and Yusaku Sako.


Bugs: AMBARI-8094
    https://issues.apache.org/jira/browse/AMBARI-8094


Repository: ambari


Description
-------

STR:
Attempted to deploy a cluster.
Install failed.
Went back to a previous step using the left nav.
It gave me the pop-up warning that I would lose work to date, I said ok, and it did nothing. The popup went away and the display remained on the Install Results page.


Diffs (updated)
-----

  ambari-web/app/router.js 97fc560 
  ambari-web/app/routes/installer.js 5c164fe 

Diff: https://reviews.apache.org/r/27471/diff/


Testing
-------

tested e2e on the cluster that had the repro for this issue


Thanks,

Jaimin Jetly


Re: Review Request 27471: Install Wizard completely breaks after install fails and going back to a previous step

Posted by Yusaku Sako <yu...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27471/#review59506
-----------------------------------------------------------


Not a fan of keeping two objects in sync explicitly.  Let's use / create a wrapper function.
Also, if this is a temporary fix, let's make a comment saying a proper refactor is TODO.

- Yusaku Sako


On Nov. 1, 2014, 6:48 p.m., Jaimin Jetly wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27471/
> -----------------------------------------------------------
> 
> (Updated Nov. 1, 2014, 6:48 p.m.)
> 
> 
> Review request for Ambari, Srimanth Gunturi and Yusaku Sako.
> 
> 
> Bugs: AMBARI-8094
>     https://issues.apache.org/jira/browse/AMBARI-8094
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> STR:
> Attempted to deploy a cluster.
> Install failed.
> Went back to a previous step using the left nav.
> It gave me the pop-up warning that I would lose work to date, I said ok, and it did nothing. The popup went away and the display remained on the Install Results page.
> 
> 
> Diffs
> -----
> 
>   ambari-web/app/router.js 97fc560 
>   ambari-web/app/routes/installer.js 5c164fe 
> 
> Diff: https://reviews.apache.org/r/27471/diff/
> 
> 
> Testing
> -------
> 
> tested e2e on the cluster that had the repro for this issue
> 
> 
> Thanks,
> 
> Jaimin Jetly
> 
>


Re: Review Request 27471: Install Wizard completely breaks after install fails and going back to a previous step

Posted by Jaimin Jetly <ja...@hortonworks.com>.

> On Nov. 1, 2014, 7:23 p.m., Yusaku Sako wrote:
> > ambari-web/app/routes/installer.js, line 66
> > <https://reviews.apache.org/r/27471/diff/1/?file=746668#file746668line66>
> >
> >     Same as above.

Because we are overriding entire localdb from persist response at https://git-wip-us.apache.org/repos/asf/ambari/repo?p=ambari.git;a=blob;f=ambari-web/app/router.js;h=97fc560f78cf88537016cc682134950288c104f1;hb=HEAD#l316 and at https://git-wip-us.apache.org/repos/asf/ambari/repo?p=ambari.git;a=blob;f=ambari-web/app/routes/installer.js;h=63d99d2721fb11018de8136b9d48a519c65c99a7;hb=HEAD#l56.

If as a pre-condition authenticated flag is set to false in the persist store then even if user successfully authenticates, the code will set it back to false and goind ahead router will not be able to bind wizard step controller with the corresponding view in the {{outlet}} due to https://git-wip-us.apache.org/repos/asf/ambari/repo?p=ambari.git;a=blob;f=ambari-web/app/controllers/installer.js;h=f0a2146945d38cbdbab94480c2bbd3d9186dce48;hb=HEAD#l95


> On Nov. 1, 2014, 7:23 p.m., Yusaku Sako wrote:
> > ambari-web/app/routes/installer.js, line 81
> > <https://reviews.apache.org/r/27471/diff/1/?file=746668#file746668line81>
> >
> >     Same as above.

same reason as above


> On Nov. 1, 2014, 7:23 p.m., Yusaku Sako wrote:
> > ambari-web/app/routes/installer.js, line 55
> > <https://reviews.apache.org/r/27471/diff/1/?file=746668#file746668line55>
> >
> >     Why do we need to set this explicitly?

This is done expicitly in the login function in the router as of now. This is when user logs in at the first time.
Unfortunatel there are places in the code where we are overriding the entire localdb with the response received from persist API which also overrides this information

This function is checking if the server is still accessible (sessionId has not expired and server is responding. At this time we are upadating the inmemory variable this.set('loggedIn', true); but not updating the localstorage variable)
The change in the success and error callback makes sure to update the authentication status  even in the localStorage


- Jaimin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27471/#review59490
-----------------------------------------------------------


On Nov. 1, 2014, 6:48 p.m., Jaimin Jetly wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27471/
> -----------------------------------------------------------
> 
> (Updated Nov. 1, 2014, 6:48 p.m.)
> 
> 
> Review request for Ambari, Srimanth Gunturi and Yusaku Sako.
> 
> 
> Bugs: AMBARI-8094
>     https://issues.apache.org/jira/browse/AMBARI-8094
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> STR:
> Attempted to deploy a cluster.
> Install failed.
> Went back to a previous step using the left nav.
> It gave me the pop-up warning that I would lose work to date, I said ok, and it did nothing. The popup went away and the display remained on the Install Results page.
> 
> 
> Diffs
> -----
> 
>   ambari-web/app/router.js 97fc560 
>   ambari-web/app/routes/installer.js 5c164fe 
> 
> Diff: https://reviews.apache.org/r/27471/diff/
> 
> 
> Testing
> -------
> 
> tested e2e on the cluster that had the repro for this issue
> 
> 
> Thanks,
> 
> Jaimin Jetly
> 
>


Re: Review Request 27471: Install Wizard completely breaks after install fails and going back to a previous step

Posted by Yusaku Sako <yu...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27471/#review59490
-----------------------------------------------------------



ambari-web/app/routes/installer.js
<https://reviews.apache.org/r/27471/#comment100772>

    Why do we need to set this explicitly?



ambari-web/app/routes/installer.js
<https://reviews.apache.org/r/27471/#comment100773>

    Same as above.



ambari-web/app/routes/installer.js
<https://reviews.apache.org/r/27471/#comment100774>

    Same as above.


- Yusaku Sako


On Nov. 1, 2014, 6:48 p.m., Jaimin Jetly wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27471/
> -----------------------------------------------------------
> 
> (Updated Nov. 1, 2014, 6:48 p.m.)
> 
> 
> Review request for Ambari, Srimanth Gunturi and Yusaku Sako.
> 
> 
> Bugs: AMBARI-8094
>     https://issues.apache.org/jira/browse/AMBARI-8094
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> STR:
> Attempted to deploy a cluster.
> Install failed.
> Went back to a previous step using the left nav.
> It gave me the pop-up warning that I would lose work to date, I said ok, and it did nothing. The popup went away and the display remained on the Install Results page.
> 
> 
> Diffs
> -----
> 
>   ambari-web/app/router.js 97fc560 
>   ambari-web/app/routes/installer.js 5c164fe 
> 
> Diff: https://reviews.apache.org/r/27471/diff/
> 
> 
> Testing
> -------
> 
> tested e2e on the cluster that had the repro for this issue
> 
> 
> Thanks,
> 
> Jaimin Jetly
> 
>