You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Jason Golieb <jg...@hortonworks.com> on 2017/11/30 21:01:43 UTC
Review Request 64225: Implemented additional functionality on the
Configure Download screen.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64225/
-----------------------------------------------------------
Review request for Ambari, Alexandr Antonenko, Andrii Tkach, Denys Buzhor, Ishan Bhatt, Jaimin Jetly, Vivek Ratnavel Subramanian, and Yusaku Sako.
Bugs: AMBARI-22569
https://issues.apache.org/jira/browse/AMBARI-22569
Repository: ambari
Description
-------
Enabled the Local Repo radio button.
Implemented the "Use Red Hat Satellite/Spacewalk" checkbox.
Implemented the "Use Proxy" checkbox and related proxy settings fields.
Diffs
-----
ambari-server/src/main/resources/common-services/ZEPPELIN/0.6.0.3.0/metainfo.xml PRE-CREATION
ambari-web/app/assets/test/tests.js b60e17a3e2
ambari-web/app/controllers/wizard/configureDownload_controller.js 0246ad0602
ambari-web/app/messages.js e014d4a012
ambari-web/app/routes/installer.js 73c716b7a9
ambari-web/app/templates/wizard/configureDownload.hbs b62a4d935c
ambari-web/app/utils.js 509efc15fa
ambari-web/app/utils/constants.js PRE-CREATION
ambari-web/app/views/wizard/configureDownload_view.js 8a02c71d6c
ambari-web/test/controllers/wizard/configureDownload_test.js PRE-CREATION
ambari-web/test/views/wizard/configureDownload_view_test.js PRE-CREATION
Diff: https://reviews.apache.org/r/64225/diff/1/
Testing
-------
20297 passing (21s)
125 pending
Thanks,
Jason Golieb
Re: Review Request 64225: Implemented additional functionality on the
Configure Download screen.
Posted by Andrii Tkach <at...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64225/#review192710
-----------------------------------------------------------
Ship it!
Ship It!
- Andrii Tkach
On Dec. 1, 2017, 4:28 p.m., Jason Golieb wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64225/
> -----------------------------------------------------------
>
> (Updated Dec. 1, 2017, 4:28 p.m.)
>
>
> Review request for Ambari, Alexandr Antonenko, Andrii Tkach, Denys Buzhor, Ishan Bhatt, Jaimin Jetly, Vivek Ratnavel Subramanian, and Yusaku Sako.
>
>
> Bugs: AMBARI-22569
> https://issues.apache.org/jira/browse/AMBARI-22569
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Enabled the Local Repo radio button.
> Implemented the "Use Red Hat Satellite/Spacewalk" checkbox.
> Implemented the "Use Proxy" checkbox and related proxy settings fields.
>
>
> Diffs
> -----
>
> ambari-server/src/main/resources/common-services/ZEPPELIN/0.6.0.3.0/metainfo.xml PRE-CREATION
> ambari-web/app/assets/test/tests.js b60e17a3e2
> ambari-web/app/controllers/wizard/configureDownload_controller.js 0246ad0602
> ambari-web/app/messages.js e014d4a012
> ambari-web/app/routes/installer.js 73c716b7a9
> ambari-web/app/templates/wizard/configureDownload.hbs b62a4d935c
> ambari-web/app/utils.js 509efc15fa
> ambari-web/app/utils/constants.js PRE-CREATION
> ambari-web/app/views/wizard/configureDownload_view.js 8a02c71d6c
> ambari-web/test/controllers/wizard/configureDownload_test.js PRE-CREATION
> ambari-web/test/views/wizard/configureDownload_view_test.js PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/64225/diff/2/
>
>
> Testing
> -------
>
> 20297 passing (21s)
> 125 pending
>
>
> Thanks,
>
> Jason Golieb
>
>
Re: Review Request 64225: Implemented additional functionality on the
Configure Download screen.
Posted by Jason Golieb <jg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64225/
-----------------------------------------------------------
(Updated Dec. 1, 2017, 4:28 p.m.)
Review request for Ambari, Alexandr Antonenko, Andrii Tkach, Denys Buzhor, Ishan Bhatt, Jaimin Jetly, Vivek Ratnavel Subramanian, and Yusaku Sako.
Changes
-------
Removed change handler hack and used changeBinding instead.
Bugs: AMBARI-22569
https://issues.apache.org/jira/browse/AMBARI-22569
Repository: ambari
Description
-------
Enabled the Local Repo radio button.
Implemented the "Use Red Hat Satellite/Spacewalk" checkbox.
Implemented the "Use Proxy" checkbox and related proxy settings fields.
Diffs (updated)
-----
ambari-server/src/main/resources/common-services/ZEPPELIN/0.6.0.3.0/metainfo.xml PRE-CREATION
ambari-web/app/assets/test/tests.js b60e17a3e2
ambari-web/app/controllers/wizard/configureDownload_controller.js 0246ad0602
ambari-web/app/messages.js e014d4a012
ambari-web/app/routes/installer.js 73c716b7a9
ambari-web/app/templates/wizard/configureDownload.hbs b62a4d935c
ambari-web/app/utils.js 509efc15fa
ambari-web/app/utils/constants.js PRE-CREATION
ambari-web/app/views/wizard/configureDownload_view.js 8a02c71d6c
ambari-web/test/controllers/wizard/configureDownload_test.js PRE-CREATION
ambari-web/test/views/wizard/configureDownload_view_test.js PRE-CREATION
Diff: https://reviews.apache.org/r/64225/diff/2/
Changes: https://reviews.apache.org/r/64225/diff/1-2/
Testing
-------
20297 passing (21s)
125 pending
Thanks,
Jason Golieb
Re: Review Request 64225: Implemented additional functionality on the
Configure Download screen.
Posted by Jason Golieb <jg...@hortonworks.com>.
> On Dec. 1, 2017, 11:05 a.m., Andrii Tkach wrote:
> > ambari-web/app/routes/installer.js
> > Lines 291 (patched)
> > <https://reviews.apache.org/r/64225/diff/1/?file=1904854#file1904854line291>
> >
> > Could you elaborate on your problem, cause Ember upgrade requires a huge change in the codebase and not feasible in near future?
>
> Jason Golieb wrote:
> OK, first let me say that I spent several hours trying to avoid this workaround and I was surprised that something that should be simple was not. The scenario is that I need to have a text field with its value bound to a field on the controller. I also need to have a change event handler attached to the field that **only responds to UI changes**, which is how a normal DOM event handler would work. There are two options that I have seen for implementing a text field in this code base:
>
> 1. Use a regular `<input>` tag. With this version, I was able to add the event handler using `{{action}}`. However, and this is the really strange part, I was unable to get the value to bind to the controller. I tried a couple variations of `{{bindAttr value="..."}}` but it did not work.
>
> 2. Use `{{view Em.TextField}}`. This has no problem binding the value. However, it does not support adding an event handler to the change event. (Later versions of Ember seem to support this.)
>
> My solution was to go with #2, and then create an observer of the bound value on the controller to perform the change event handling. However, as this is observing the controller directly, it reacts to ALL changes to the controller value, not just those coming from the UI. That means it gets fired on the initialization of the value from the database as well, which is not desirable. Therefore, I had to add a Boolean flag to temporarily prevent the observer from doing anything during the initial load of the screen.
>
> I do not expect us to upgrade Ember any time soon, but I wanted to leave the comment in the code for future understanding.
>
> Andrii Tkach wrote:
> I think that's what you need:
> {{view Em.TextField changeBinding="yourFunc"}}
> Or you can just validate value so after controller initialization it will be rejected and after user input will be applied.
I thought I had tried changeBinding before, but I tried it again and it worked. Thanks! I've updated the patch.
- Jason
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64225/#review192466
-----------------------------------------------------------
On Dec. 1, 2017, 4:28 p.m., Jason Golieb wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64225/
> -----------------------------------------------------------
>
> (Updated Dec. 1, 2017, 4:28 p.m.)
>
>
> Review request for Ambari, Alexandr Antonenko, Andrii Tkach, Denys Buzhor, Ishan Bhatt, Jaimin Jetly, Vivek Ratnavel Subramanian, and Yusaku Sako.
>
>
> Bugs: AMBARI-22569
> https://issues.apache.org/jira/browse/AMBARI-22569
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Enabled the Local Repo radio button.
> Implemented the "Use Red Hat Satellite/Spacewalk" checkbox.
> Implemented the "Use Proxy" checkbox and related proxy settings fields.
>
>
> Diffs
> -----
>
> ambari-server/src/main/resources/common-services/ZEPPELIN/0.6.0.3.0/metainfo.xml PRE-CREATION
> ambari-web/app/assets/test/tests.js b60e17a3e2
> ambari-web/app/controllers/wizard/configureDownload_controller.js 0246ad0602
> ambari-web/app/messages.js e014d4a012
> ambari-web/app/routes/installer.js 73c716b7a9
> ambari-web/app/templates/wizard/configureDownload.hbs b62a4d935c
> ambari-web/app/utils.js 509efc15fa
> ambari-web/app/utils/constants.js PRE-CREATION
> ambari-web/app/views/wizard/configureDownload_view.js 8a02c71d6c
> ambari-web/test/controllers/wizard/configureDownload_test.js PRE-CREATION
> ambari-web/test/views/wizard/configureDownload_view_test.js PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/64225/diff/2/
>
>
> Testing
> -------
>
> 20297 passing (21s)
> 125 pending
>
>
> Thanks,
>
> Jason Golieb
>
>
Re: Review Request 64225: Implemented additional functionality on the
Configure Download screen.
Posted by Jason Golieb <jg...@hortonworks.com>.
> On Dec. 1, 2017, 11:05 a.m., Andrii Tkach wrote:
> > ambari-web/app/routes/installer.js
> > Lines 291 (patched)
> > <https://reviews.apache.org/r/64225/diff/1/?file=1904854#file1904854line291>
> >
> > Could you elaborate on your problem, cause Ember upgrade requires a huge change in the codebase and not feasible in near future?
OK, first let me say that I spent several hours trying to avoid this workaround and I was surprised that something that should be simple was not. The scenario is that I need to have a text field with its value bound to a field on the controller. I also need to have a change event handler attached to the field that **only responds to UI changes**, which is how a normal DOM event handler would work. There are two options that I have seen for implementing a text field in this code base:
1. Use a regular `<input>` tag. With this version, I was able to add the event handler using `{{action}}`. However, and this is the really strange part, I was unable to get the value to bind to the controller. I tried a couple variations of `{{bindAttr value="..."}}` but it did not work.
2. Use `{{view Em.TextField}}`. This has no problem binding the value. However, it does not support adding an event handler to the change event. (Later versions of Ember seem to support this.)
My solution was to go with #2, and then create an observer of the bound value on the controller to perform the change event handling. However, as this is observing the controller directly, it reacts to ALL changes to the controller value, not just those coming from the UI. That means it gets fired on the initialization of the value from the database as well, which is not desirable. Therefore, I had to add a Boolean flag to temporarily prevent the observer from doing anything during the initial load of the screen.
I do not expect us to upgrade Ember any time soon, but I wanted to leave the comment in the code for future understanding.
- Jason
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64225/#review192466
-----------------------------------------------------------
On Nov. 30, 2017, 9:01 p.m., Jason Golieb wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64225/
> -----------------------------------------------------------
>
> (Updated Nov. 30, 2017, 9:01 p.m.)
>
>
> Review request for Ambari, Alexandr Antonenko, Andrii Tkach, Denys Buzhor, Ishan Bhatt, Jaimin Jetly, Vivek Ratnavel Subramanian, and Yusaku Sako.
>
>
> Bugs: AMBARI-22569
> https://issues.apache.org/jira/browse/AMBARI-22569
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Enabled the Local Repo radio button.
> Implemented the "Use Red Hat Satellite/Spacewalk" checkbox.
> Implemented the "Use Proxy" checkbox and related proxy settings fields.
>
>
> Diffs
> -----
>
> ambari-server/src/main/resources/common-services/ZEPPELIN/0.6.0.3.0/metainfo.xml PRE-CREATION
> ambari-web/app/assets/test/tests.js b60e17a3e2
> ambari-web/app/controllers/wizard/configureDownload_controller.js 0246ad0602
> ambari-web/app/messages.js e014d4a012
> ambari-web/app/routes/installer.js 73c716b7a9
> ambari-web/app/templates/wizard/configureDownload.hbs b62a4d935c
> ambari-web/app/utils.js 509efc15fa
> ambari-web/app/utils/constants.js PRE-CREATION
> ambari-web/app/views/wizard/configureDownload_view.js 8a02c71d6c
> ambari-web/test/controllers/wizard/configureDownload_test.js PRE-CREATION
> ambari-web/test/views/wizard/configureDownload_view_test.js PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/64225/diff/1/
>
>
> Testing
> -------
>
> 20297 passing (21s)
> 125 pending
>
>
> Thanks,
>
> Jason Golieb
>
>
Re: Review Request 64225: Implemented additional functionality on the
Configure Download screen.
Posted by Andrii Tkach <at...@hortonworks.com>.
> On Dec. 1, 2017, 11:05 a.m., Andrii Tkach wrote:
> > ambari-web/app/routes/installer.js
> > Lines 291 (patched)
> > <https://reviews.apache.org/r/64225/diff/1/?file=1904854#file1904854line291>
> >
> > Could you elaborate on your problem, cause Ember upgrade requires a huge change in the codebase and not feasible in near future?
>
> Jason Golieb wrote:
> OK, first let me say that I spent several hours trying to avoid this workaround and I was surprised that something that should be simple was not. The scenario is that I need to have a text field with its value bound to a field on the controller. I also need to have a change event handler attached to the field that **only responds to UI changes**, which is how a normal DOM event handler would work. There are two options that I have seen for implementing a text field in this code base:
>
> 1. Use a regular `<input>` tag. With this version, I was able to add the event handler using `{{action}}`. However, and this is the really strange part, I was unable to get the value to bind to the controller. I tried a couple variations of `{{bindAttr value="..."}}` but it did not work.
>
> 2. Use `{{view Em.TextField}}`. This has no problem binding the value. However, it does not support adding an event handler to the change event. (Later versions of Ember seem to support this.)
>
> My solution was to go with #2, and then create an observer of the bound value on the controller to perform the change event handling. However, as this is observing the controller directly, it reacts to ALL changes to the controller value, not just those coming from the UI. That means it gets fired on the initialization of the value from the database as well, which is not desirable. Therefore, I had to add a Boolean flag to temporarily prevent the observer from doing anything during the initial load of the screen.
>
> I do not expect us to upgrade Ember any time soon, but I wanted to leave the comment in the code for future understanding.
I think that's what you need:
{{view Em.TextField changeBinding="yourFunc"}}
Or you can just validate value so after controller initialization it will be rejected and after user input will be applied.
- Andrii
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64225/#review192466
-----------------------------------------------------------
On Nov. 30, 2017, 9:01 p.m., Jason Golieb wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64225/
> -----------------------------------------------------------
>
> (Updated Nov. 30, 2017, 9:01 p.m.)
>
>
> Review request for Ambari, Alexandr Antonenko, Andrii Tkach, Denys Buzhor, Ishan Bhatt, Jaimin Jetly, Vivek Ratnavel Subramanian, and Yusaku Sako.
>
>
> Bugs: AMBARI-22569
> https://issues.apache.org/jira/browse/AMBARI-22569
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Enabled the Local Repo radio button.
> Implemented the "Use Red Hat Satellite/Spacewalk" checkbox.
> Implemented the "Use Proxy" checkbox and related proxy settings fields.
>
>
> Diffs
> -----
>
> ambari-server/src/main/resources/common-services/ZEPPELIN/0.6.0.3.0/metainfo.xml PRE-CREATION
> ambari-web/app/assets/test/tests.js b60e17a3e2
> ambari-web/app/controllers/wizard/configureDownload_controller.js 0246ad0602
> ambari-web/app/messages.js e014d4a012
> ambari-web/app/routes/installer.js 73c716b7a9
> ambari-web/app/templates/wizard/configureDownload.hbs b62a4d935c
> ambari-web/app/utils.js 509efc15fa
> ambari-web/app/utils/constants.js PRE-CREATION
> ambari-web/app/views/wizard/configureDownload_view.js 8a02c71d6c
> ambari-web/test/controllers/wizard/configureDownload_test.js PRE-CREATION
> ambari-web/test/views/wizard/configureDownload_view_test.js PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/64225/diff/1/
>
>
> Testing
> -------
>
> 20297 passing (21s)
> 125 pending
>
>
> Thanks,
>
> Jason Golieb
>
>
Re: Review Request 64225: Implemented additional functionality on the
Configure Download screen.
Posted by Andrii Tkach <at...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64225/#review192466
-----------------------------------------------------------
ambari-web/app/routes/installer.js
Lines 291 (patched)
<https://reviews.apache.org/r/64225/#comment270620>
Could you elaborate on your problem, cause Ember upgrade requires a huge change in the codebase and not feasible in near future?
- Andrii Tkach
On Nov. 30, 2017, 9:01 p.m., Jason Golieb wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64225/
> -----------------------------------------------------------
>
> (Updated Nov. 30, 2017, 9:01 p.m.)
>
>
> Review request for Ambari, Alexandr Antonenko, Andrii Tkach, Denys Buzhor, Ishan Bhatt, Jaimin Jetly, Vivek Ratnavel Subramanian, and Yusaku Sako.
>
>
> Bugs: AMBARI-22569
> https://issues.apache.org/jira/browse/AMBARI-22569
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Enabled the Local Repo radio button.
> Implemented the "Use Red Hat Satellite/Spacewalk" checkbox.
> Implemented the "Use Proxy" checkbox and related proxy settings fields.
>
>
> Diffs
> -----
>
> ambari-server/src/main/resources/common-services/ZEPPELIN/0.6.0.3.0/metainfo.xml PRE-CREATION
> ambari-web/app/assets/test/tests.js b60e17a3e2
> ambari-web/app/controllers/wizard/configureDownload_controller.js 0246ad0602
> ambari-web/app/messages.js e014d4a012
> ambari-web/app/routes/installer.js 73c716b7a9
> ambari-web/app/templates/wizard/configureDownload.hbs b62a4d935c
> ambari-web/app/utils.js 509efc15fa
> ambari-web/app/utils/constants.js PRE-CREATION
> ambari-web/app/views/wizard/configureDownload_view.js 8a02c71d6c
> ambari-web/test/controllers/wizard/configureDownload_test.js PRE-CREATION
> ambari-web/test/views/wizard/configureDownload_view_test.js PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/64225/diff/1/
>
>
> Testing
> -------
>
> 20297 passing (21s)
> 125 pending
>
>
> Thanks,
>
> Jason Golieb
>
>