You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Dmitro Lisnichenko <dl...@hortonworks.com> on 2017/08/28 15:49:17 UTC
Re: Review Request 61715: PREVIEW: package_regex in
get_package_from_available() can match wrong pkg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61715/
-----------------------------------------------------------
(Updated Aug. 28, 2017, 6:49 p.m.)
Review request for Ambari, Dmytro Grinenko and Vitalyi Brodetskyi.
Changes
-------
Preview of a patch with new approach
Previous patch has been reverted some time ago
Summary (updated)
-----------------
PREVIEW: package_regex in get_package_from_available() can match wrong pkg
Bugs: AMBARI-21744
https://issues.apache.org/jira/browse/AMBARI-21744
Repository: ambari
Description
-------
Due to the issue with regex (missing ^ and $ boundaries), resource_management.libraries.script.script.Script#get_package_from_available may return wrong package.
{code}
>>> list=['hbase_3_0_0_0_229-master', 'hbase_3_0_0_0_229']
>>> if re.match('hbase_(\d|_)+', 'hbase_3_0_0_0_229-master'):
... print 'YES'
...
YES
>>> if re.match('hbase_(\d|_)+', 'hbase_3_0_0_0_229'):
... print 'YES'
...
YES
{code}
In this case, the first package name from a list of available packages will be returned.
The impact of bug is that we may install a wrong package if it's simillary named and goes first at list. Patch is a single-line fix.
Diffs (updated)
-----
ambari-common/src/main/python/resource_management/libraries/script/script.py 313b666878
Diff: https://reviews.apache.org/r/61715/diff/2/
Changes: https://reviews.apache.org/r/61715/diff/1-2/
Testing (updated)
-------
Will fix unit tests a bit later
Thanks,
Dmitro Lisnichenko
Re: Review Request 61715: package_regex in
get_package_from_available() can match wrong pkg
Posted by Dmitro Lisnichenko <dl...@hortonworks.com>.
> On Aug. 28, 2017, 6:56 p.m., Dmytro Grinenko wrote:
> > ambari-common/src/main/python/resource_management/libraries/script/script.py
> > Line 455 (original), 455 (patched)
> > <https://reviews.apache.org/r/61715/diff/2/?file=1806123#file1806123line455>
> >
> > it looks that this function is cross-linux version, could we move it to https://github.com/apache/ambari/tree/trunk/ambari-common/src/main/python/resource_management/core/providers/package to respective os provider?
Returned to simpler regex
- Dmitro
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61715/#review183939
-----------------------------------------------------------
On Aug. 29, 2017, 6:07 p.m., Dmitro Lisnichenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61715/
> -----------------------------------------------------------
>
> (Updated Aug. 29, 2017, 6:07 p.m.)
>
>
> Review request for Ambari, Dmytro Grinenko and Vitalyi Brodetskyi.
>
>
> Bugs: AMBARI-21744
> https://issues.apache.org/jira/browse/AMBARI-21744
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Due to the issue with regex (missing ^ and $ boundaries), resource_management.libraries.script.script.Script#get_package_from_available may return wrong package.
> {code}
> >>> list=['hbase_3_0_0_0_229-master', 'hbase_3_0_0_0_229']
> >>> if re.match('hbase_(\d|_)+', 'hbase_3_0_0_0_229-master'):
> ... print 'YES'
> ...
> YES
> >>> if re.match('hbase_(\d|_)+', 'hbase_3_0_0_0_229'):
> ... print 'YES'
> ...
> YES
> {code}
>
> In this case, the first package name from a list of available packages will be returned.
> The impact of bug is that we may install a wrong package if it's simillary named and goes first at list. Patch is a single-line fix.
>
>
> Diffs
> -----
>
> ambari-common/src/main/python/resource_management/libraries/script/script.py 313b666878
> ambari-server/src/test/python/custom_actions/TestInstallPackages.py de2cced554
>
>
> Diff: https://reviews.apache.org/r/61715/diff/3/
>
>
> Testing
> -------
>
> mvn clean test
>
>
> Thanks,
>
> Dmitro Lisnichenko
>
>
Re: Review Request 61715: PREVIEW: package_regex in
get_package_from_available() can match wrong pkg
Posted by Dmytro Grinenko <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61715/#review183939
-----------------------------------------------------------
ambari-common/src/main/python/resource_management/libraries/script/script.py
Line 455 (original), 455 (patched)
<https://reviews.apache.org/r/61715/#comment259985>
it looks that this function is cross-linux version, could we move it to https://github.com/apache/ambari/tree/trunk/ambari-common/src/main/python/resource_management/core/providers/package to respective os provider?
- Dmytro Grinenko
On Aug. 28, 2017, 3:49 p.m., Dmitro Lisnichenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61715/
> -----------------------------------------------------------
>
> (Updated Aug. 28, 2017, 3:49 p.m.)
>
>
> Review request for Ambari, Dmytro Grinenko and Vitalyi Brodetskyi.
>
>
> Bugs: AMBARI-21744
> https://issues.apache.org/jira/browse/AMBARI-21744
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Due to the issue with regex (missing ^ and $ boundaries), resource_management.libraries.script.script.Script#get_package_from_available may return wrong package.
> {code}
> >>> list=['hbase_3_0_0_0_229-master', 'hbase_3_0_0_0_229']
> >>> if re.match('hbase_(\d|_)+', 'hbase_3_0_0_0_229-master'):
> ... print 'YES'
> ...
> YES
> >>> if re.match('hbase_(\d|_)+', 'hbase_3_0_0_0_229'):
> ... print 'YES'
> ...
> YES
> {code}
>
> In this case, the first package name from a list of available packages will be returned.
> The impact of bug is that we may install a wrong package if it's simillary named and goes first at list. Patch is a single-line fix.
>
>
> Diffs
> -----
>
> ambari-common/src/main/python/resource_management/libraries/script/script.py 313b666878
>
>
> Diff: https://reviews.apache.org/r/61715/diff/2/
>
>
> Testing
> -------
>
> Will fix unit tests a bit later
>
>
> Thanks,
>
> Dmitro Lisnichenko
>
>
Re: Review Request 61715: package_regex in
get_package_from_available() can match wrong pkg
Posted by Dmytro Grinenko <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61715/#review184142
-----------------------------------------------------------
Ship it!
Ship It!
- Dmytro Grinenko
On Aug. 29, 2017, 3:07 p.m., Dmitro Lisnichenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61715/
> -----------------------------------------------------------
>
> (Updated Aug. 29, 2017, 3:07 p.m.)
>
>
> Review request for Ambari, Dmytro Grinenko and Vitalyi Brodetskyi.
>
>
> Bugs: AMBARI-21744
> https://issues.apache.org/jira/browse/AMBARI-21744
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Due to the issue with regex (missing ^ and $ boundaries), resource_management.libraries.script.script.Script#get_package_from_available may return wrong package.
> {code}
> >>> list=['hbase_3_0_0_0_229-master', 'hbase_3_0_0_0_229']
> >>> if re.match('hbase_(\d|_)+', 'hbase_3_0_0_0_229-master'):
> ... print 'YES'
> ...
> YES
> >>> if re.match('hbase_(\d|_)+', 'hbase_3_0_0_0_229'):
> ... print 'YES'
> ...
> YES
> {code}
>
> In this case, the first package name from a list of available packages will be returned.
> The impact of bug is that we may install a wrong package if it's simillary named and goes first at list. Patch is a single-line fix.
>
>
> Diffs
> -----
>
> ambari-common/src/main/python/resource_management/libraries/script/script.py 313b666878
> ambari-server/src/test/python/custom_actions/TestInstallPackages.py de2cced554
>
>
> Diff: https://reviews.apache.org/r/61715/diff/3/
>
>
> Testing
> -------
>
> mvn clean test
>
>
> Thanks,
>
> Dmitro Lisnichenko
>
>
Re: Review Request 61715: package_regex in
get_package_from_available() can match wrong pkg
Posted by Dmitro Lisnichenko <dl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61715/
-----------------------------------------------------------
(Updated Aug. 29, 2017, 6:07 p.m.)
Review request for Ambari, Dmytro Grinenko and Vitalyi Brodetskyi.
Changes
-------
Returned to simpler regex because Dmytro Grinenko already fixed striping out of package architecture
Summary (updated)
-----------------
package_regex in get_package_from_available() can match wrong pkg
Bugs: AMBARI-21744
https://issues.apache.org/jira/browse/AMBARI-21744
Repository: ambari
Description
-------
Due to the issue with regex (missing ^ and $ boundaries), resource_management.libraries.script.script.Script#get_package_from_available may return wrong package.
{code}
>>> list=['hbase_3_0_0_0_229-master', 'hbase_3_0_0_0_229']
>>> if re.match('hbase_(\d|_)+', 'hbase_3_0_0_0_229-master'):
... print 'YES'
...
YES
>>> if re.match('hbase_(\d|_)+', 'hbase_3_0_0_0_229'):
... print 'YES'
...
YES
{code}
In this case, the first package name from a list of available packages will be returned.
The impact of bug is that we may install a wrong package if it's simillary named and goes first at list. Patch is a single-line fix.
Diffs (updated)
-----
ambari-common/src/main/python/resource_management/libraries/script/script.py 313b666878
ambari-server/src/test/python/custom_actions/TestInstallPackages.py de2cced554
Diff: https://reviews.apache.org/r/61715/diff/3/
Changes: https://reviews.apache.org/r/61715/diff/2-3/
Testing (updated)
-------
mvn clean test
Thanks,
Dmitro Lisnichenko
Re: Review Request 61715: PREVIEW: package_regex in
get_package_from_available() can match wrong pkg
Posted by Dmitro Lisnichenko <dl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61715/#review183940
-----------------------------------------------------------
ambari-common/src/main/python/resource_management/libraries/script/script.py
Line 455 (original), 455 (patched)
<https://reviews.apache.org/r/61715/#comment259986>
Matches package name with substituted build number.
This substring should end with either:
- a dot (dot separates package name and architecture under Centos/Suse)
- a colon (colon separates package name and architecture under Ubuntu)
- end of string
- Dmitro Lisnichenko
On Aug. 28, 2017, 6:49 p.m., Dmitro Lisnichenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61715/
> -----------------------------------------------------------
>
> (Updated Aug. 28, 2017, 6:49 p.m.)
>
>
> Review request for Ambari, Dmytro Grinenko and Vitalyi Brodetskyi.
>
>
> Bugs: AMBARI-21744
> https://issues.apache.org/jira/browse/AMBARI-21744
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Due to the issue with regex (missing ^ and $ boundaries), resource_management.libraries.script.script.Script#get_package_from_available may return wrong package.
> {code}
> >>> list=['hbase_3_0_0_0_229-master', 'hbase_3_0_0_0_229']
> >>> if re.match('hbase_(\d|_)+', 'hbase_3_0_0_0_229-master'):
> ... print 'YES'
> ...
> YES
> >>> if re.match('hbase_(\d|_)+', 'hbase_3_0_0_0_229'):
> ... print 'YES'
> ...
> YES
> {code}
>
> In this case, the first package name from a list of available packages will be returned.
> The impact of bug is that we may install a wrong package if it's simillary named and goes first at list. Patch is a single-line fix.
>
>
> Diffs
> -----
>
> ambari-common/src/main/python/resource_management/libraries/script/script.py 313b666878
>
>
> Diff: https://reviews.apache.org/r/61715/diff/2/
>
>
> Testing
> -------
>
> Will fix unit tests a bit later
>
>
> Thanks,
>
> Dmitro Lisnichenko
>
>