You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Graham Wallis <gr...@uk.ibm.com> on 2017/04/13 16:28:17 UTC
Review Request 58422: Changed signal handling in atlas_stop.py for
Windows
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58422/
-----------------------------------------------------------
Review request for atlas and David Radley.
Repository: atlas
Description
-------
On Windows there is no SIGKILL in the python signal library, so attempting to import SIGKILL causes a fatal error.
This change introduces a platform switch that avoids trying to import SIGKILL and if the 30sec timeout expires
it avoids attempting to use SIGKILL, instead using SIGTERM as the os.kill() function on Windows will terminate
the process and should be as severe as a SIGKILL (kill -9) on a *nix system.
Diffs
-----
distro/src/bin/atlas_stop.py a25d25aee599e7cc9ca3caaff8ff7f11b7e0c789
Diff: https://reviews.apache.org/r/58422/diff/1/
Testing
-------
Functional testing manually verified
Thanks,
Graham Wallis
Re: Review Request 58422: Changed signal handling in atlas_stop.py for
Windows
Posted by David Radley <da...@uk.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58422/#review172612
-----------------------------------------------------------
Ship it!
Ship It!
- David Radley
On April 13, 2017, 4:28 p.m., Graham Wallis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58422/
> -----------------------------------------------------------
>
> (Updated April 13, 2017, 4:28 p.m.)
>
>
> Review request for atlas and David Radley.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> On Windows there is no SIGKILL in the python signal library, so attempting to import SIGKILL causes a fatal error.
> This change introduces a platform switch that avoids trying to import SIGKILL and if the 30sec timeout expires
> it avoids attempting to use SIGKILL, instead using SIGTERM as the os.kill() function on Windows will terminate
> the process and should be as severe as a SIGKILL (kill -9) on a *nix system.
>
>
> Diffs
> -----
>
> distro/src/bin/atlas_stop.py a25d25aee599e7cc9ca3caaff8ff7f11b7e0c789
>
>
> Diff: https://reviews.apache.org/r/58422/diff/1/
>
>
> Testing
> -------
>
> Functional testing manually verified
>
>
> Thanks,
>
> Graham Wallis
>
>
Re: Review Request 58422: Addressed Davids review comments
Posted by David Radley <da...@uk.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58422/#review180192
-----------------------------------------------------------
Ship it!
Ship It!
- David Radley
On July 11, 2017, 2:02 p.m., Graham Wallis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58422/
> -----------------------------------------------------------
>
> (Updated July 11, 2017, 2:02 p.m.)
>
>
> Review request for atlas and David Radley.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> On Windows there is no SIGKILL in the python signal library, so attempting to import SIGKILL causes a fatal error.
> This change introduces a platform switch that avoids trying to import SIGKILL and if the 30sec timeout expires
> it avoids attempting to use SIGKILL, instead using SIGTERM as the os.kill() function on Windows will terminate
> the process and should be as severe as a SIGKILL (kill -9) on a *nix system.
>
>
> Diffs
> -----
>
> distro/src/bin/atlas_stop.py a25d25aee599e7cc9ca3caaff8ff7f11b7e0c789
>
>
> Diff: https://reviews.apache.org/r/58422/diff/3/
>
>
> Testing
> -------
>
> Manually tested
>
>
> Thanks,
>
> Graham Wallis
>
>
Re: Review Request 58422: Addressed Davids review comments
Posted by Graham Wallis <gr...@uk.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58422/
-----------------------------------------------------------
(Updated July 11, 2017, 2:02 p.m.)
Review request for atlas and David Radley.
Repository: atlas
Description
-------
On Windows there is no SIGKILL in the python signal library, so attempting to import SIGKILL causes a fatal error.
This change introduces a platform switch that avoids trying to import SIGKILL and if the 30sec timeout expires
it avoids attempting to use SIGKILL, instead using SIGTERM as the os.kill() function on Windows will terminate
the process and should be as severe as a SIGKILL (kill -9) on a *nix system.
Diffs (updated)
-----
distro/src/bin/atlas_stop.py a25d25aee599e7cc9ca3caaff8ff7f11b7e0c789
Diff: https://reviews.apache.org/r/58422/diff/3/
Changes: https://reviews.apache.org/r/58422/diff/2-3/
Testing
-------
Manually tested
Thanks,
Graham Wallis
Re: Review Request 58422: Addressed Davids review comments
Posted by Graham Wallis <gr...@uk.ibm.com>.
> On June 27, 2017, 9:11 a.m., David Radley wrote:
> >
Please ignore the 60747 review (created by mistake). This review (58422) is the only one relevant to ATLAS-1733.
> On June 27, 2017, 9:11 a.m., David Radley wrote:
> > distro/src/bin/atlas_stop.py
> > Lines 82 (patched)
> > <https://reviews.apache.org/r/58422/diff/2/?file=1708250#file1708250line82>
> >
> > formatting error introduced in the fix
I rebased and rebuilt the patch - new review is https://reviews.apache.org/r/60747/
- Graham
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58422/#review178961
-----------------------------------------------------------
On May 4, 2017, 10:52 a.m., Graham Wallis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58422/
> -----------------------------------------------------------
>
> (Updated May 4, 2017, 10:52 a.m.)
>
>
> Review request for atlas and David Radley.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> On Windows there is no SIGKILL in the python signal library, so attempting to import SIGKILL causes a fatal error.
> This change introduces a platform switch that avoids trying to import SIGKILL and if the 30sec timeout expires
> it avoids attempting to use SIGKILL, instead using SIGTERM as the os.kill() function on Windows will terminate
> the process and should be as severe as a SIGKILL (kill -9) on a *nix system.
>
>
> Diffs
> -----
>
> distro/src/bin/atlas_stop.py a25d25aee599e7cc9ca3caaff8ff7f11b7e0c789
>
>
> Diff: https://reviews.apache.org/r/58422/diff/2/
>
>
> Testing
> -------
>
> Manually tested
>
>
> Thanks,
>
> Graham Wallis
>
>
Re: Review Request 58422: Addressed Davids review comments
Posted by David Radley <da...@uk.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58422/#review178961
-----------------------------------------------------------
distro/src/bin/atlas_stop.py
Lines 82 (patched)
<https://reviews.apache.org/r/58422/#comment253355>
formatting error introduced in the fix
- David Radley
On May 4, 2017, 10:52 a.m., Graham Wallis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58422/
> -----------------------------------------------------------
>
> (Updated May 4, 2017, 10:52 a.m.)
>
>
> Review request for atlas and David Radley.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> On Windows there is no SIGKILL in the python signal library, so attempting to import SIGKILL causes a fatal error.
> This change introduces a platform switch that avoids trying to import SIGKILL and if the 30sec timeout expires
> it avoids attempting to use SIGKILL, instead using SIGTERM as the os.kill() function on Windows will terminate
> the process and should be as severe as a SIGKILL (kill -9) on a *nix system.
>
>
> Diffs
> -----
>
> distro/src/bin/atlas_stop.py a25d25aee599e7cc9ca3caaff8ff7f11b7e0c789
>
>
> Diff: https://reviews.apache.org/r/58422/diff/2/
>
>
> Testing
> -------
>
> Manually tested
>
>
> Thanks,
>
> Graham Wallis
>
>
Re: Review Request 58422: Addressed Davids review comments
Posted by Graham Wallis <gr...@uk.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58422/
-----------------------------------------------------------
(Updated May 4, 2017, 10:52 a.m.)
Review request for atlas and David Radley.
Changes
-------
Have addressed David's review comments - added code comment and improved logged messages
Summary (updated)
-----------------
Addressed Davids review comments
Repository: atlas
Description
-------
On Windows there is no SIGKILL in the python signal library, so attempting to import SIGKILL causes a fatal error.
This change introduces a platform switch that avoids trying to import SIGKILL and if the 30sec timeout expires
it avoids attempting to use SIGKILL, instead using SIGTERM as the os.kill() function on Windows will terminate
the process and should be as severe as a SIGKILL (kill -9) on a *nix system.
Diffs (updated)
-----
distro/src/bin/atlas_stop.py a25d25aee599e7cc9ca3caaff8ff7f11b7e0c789
Diff: https://reviews.apache.org/r/58422/diff/2/
Changes: https://reviews.apache.org/r/58422/diff/1-2/
Testing (updated)
-------
Manually tested
Thanks,
Graham Wallis
Re: Review Request 58422: Changed signal handling in atlas_stop.py for
Windows
Posted by Ashutosh Mestry <am...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58422/#review171947
-----------------------------------------------------------
Ship it!
Ship It!
- Ashutosh Mestry
On April 13, 2017, 4:28 p.m., Graham Wallis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58422/
> -----------------------------------------------------------
>
> (Updated April 13, 2017, 4:28 p.m.)
>
>
> Review request for atlas and David Radley.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> On Windows there is no SIGKILL in the python signal library, so attempting to import SIGKILL causes a fatal error.
> This change introduces a platform switch that avoids trying to import SIGKILL and if the 30sec timeout expires
> it avoids attempting to use SIGKILL, instead using SIGTERM as the os.kill() function on Windows will terminate
> the process and should be as severe as a SIGKILL (kill -9) on a *nix system.
>
>
> Diffs
> -----
>
> distro/src/bin/atlas_stop.py a25d25aee599e7cc9ca3caaff8ff7f11b7e0c789
>
>
> Diff: https://reviews.apache.org/r/58422/diff/1/
>
>
> Testing
> -------
>
> Functional testing manually verified
>
>
> Thanks,
>
> Graham Wallis
>
>
Re: Review Request 58422: Changed signal handling in atlas_stop.py for
Windows
Posted by David Radley <da...@uk.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58422/#review172611
-----------------------------------------------------------
distro/src/bin/atlas_stop.py
Line 71 (original), 74 (patched)
<https://reviews.apache.org/r/58422/#comment245698>
The fix looks good. A couple of things you could change:
- you could correct the seconds seconds in the message you altered
- you could add comments to detail why this was necessary
- you could keep the signal name in the message and have 2 more specific messages.
These are my suggestions - it is up to you whether you make these changes as they are all quite minor.
- David Radley
On April 13, 2017, 4:28 p.m., Graham Wallis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58422/
> -----------------------------------------------------------
>
> (Updated April 13, 2017, 4:28 p.m.)
>
>
> Review request for atlas and David Radley.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> On Windows there is no SIGKILL in the python signal library, so attempting to import SIGKILL causes a fatal error.
> This change introduces a platform switch that avoids trying to import SIGKILL and if the 30sec timeout expires
> it avoids attempting to use SIGKILL, instead using SIGTERM as the os.kill() function on Windows will terminate
> the process and should be as severe as a SIGKILL (kill -9) on a *nix system.
>
>
> Diffs
> -----
>
> distro/src/bin/atlas_stop.py a25d25aee599e7cc9ca3caaff8ff7f11b7e0c789
>
>
> Diff: https://reviews.apache.org/r/58422/diff/1/
>
>
> Testing
> -------
>
> Functional testing manually verified
>
>
> Thanks,
>
> Graham Wallis
>
>