Re: [Starlingx-discuss] FW: [starlingx-discuss][python3] four solutions for "Add python3-daemon required by logmgmt"
+StarlingX-Discuss On 4/28/20 12:47 AM, An, Ran1 wrote:
Hi Cores:
There were some concerns on patch https://review.opendev.org/#/c/717211/, and we had a discussion on last Non-Openstack meeting few days ago.
By further investigation, there are 4 solutions below. W*e'd like to prefer option3 , if no other objection , we will implement option3*.
I am not sure I understand how option 1 and option 3 are different both are modifying upstream projects and require the StarlingX community to maintain a patched version (ie technical debt). Is there an option that modifies the logmgmt code directly instead of changing the daemon code? Sau!
*Background:*
Logmgmt uses “daemon.runner. DaemonRunner()” from package python-daemon <http://pypi.python.org/pypi/python-daemon/> to create/recreate/kill a daemon process in which it run log-rotate and control system logs size.
However, python-daemon has a “unbuffered I/O related error” in python3.x [1], which require us do some changes to upgrade logmgmt to python3.
*Solutions:*
Option 1: fix the bug of python-daemon by adding patch when we build the python3-daemon rpm. (implement in patch-717211/ <https://review.opendev.org/#/c/717211/>)
* add patch to fix the bug of python-daemon in python3 * build python3-daemon rpm from python-daemon srpm
Option 2: rewrite the entrance function of logmgmt (start_polling()) following examples provides by PEP 3143 <https://www.python.org/dev/peps/pep-3143/>
* using “daemon.Context()” instead of “daemon.runner.DaemonRunner() in entrance function of logmgmt. * implement a runner class “MyDaemonRunner()” in logmgmt * build python3-daemon rpm from python-daemon srpm
Limitation: the implementation of daemon service required to write is very similar with “daemon.runner”.
Option 3: import python-daemon-3k [2] python3 condition.
* use "python-daemon-3k" instead of "python-daemon" when the env is python3 * add patch to fix the bug of python-daemon in python3 (refer to pull request #1 from takuti/fix-unbuffered-error <https://github.com/jbvsmo/python-daemon/commit/91567c9e68a1e02d3e9a811eaf9ab49961d9d63c>) * build python3-daemon rpm from python-daemon-3k release <https://github.com/jbvsmo/python-daemon/archive/1.5.8.tar.gz>
Option 4: implement a daemon class "MyDaemon" and rewrite the logmgmt codes with "MyDaemon"
* implement a daemon class "MyDaemon" (refer to https://www.workaround.cz/howto-make-code-daemon-python-3) * rewrite the logmgmt codes with "MyDaemon"
*Comparation: *
Option
Pkg provides daemon class
Require build python3-daemon.rpm
maintain offline patch
Stars of github
Changes
Option1
python-daemon <http://pypi.python.org/pypi/python-daemon/>
Yes
yes
916
little
Option2
python-daemon <http://pypi.python.org/pypi/python-daemon/>
Yes
916
Medium
Option3
python-daemon-3K <https://pypi.org/project/python-daemon-3K>
Yes
yes
14
little
Option4
write “MyDaemon” based on basic python libs
No
No
0
huge
[1] https://pagure.io/python-daemon/issue/5; https://pagure.io/python-daemon/issue/8; https://pagure.io/python-daemon/issue/36;
[2] https://pypi.org/project/python-daemon-3K/; we had assessed 4 projects that implement the python daemon following the PEP 3143 and “python-daemon-3k” is the most proper for logmgmt.
On 2020-04-28 06:44:35 -0700 (-0700), Saul Wold wrote:
I am not sure I understand how option 1 and option 3 are different both are modifying upstream projects and require the StarlingX community to maintain a patched version (ie technical debt).
Is there an option that modifies the logmgmt code directly instead of changing the daemon code? [...]
It might be possible to subclass daemon.runner and shadow the offending methods, rather than forking the entire codebase.
Option 3: import python-daemon-3k [2] python3 condition.
* use "python-daemon-3k" instead of "python-daemon" when the env is python3 * add patch to fix the bug of python-daemon in python3 (refer to pull request #1 from takuti/fix-unbuffered-error <https://github.com/jbvsmo/python-daemon/commit/91567c9e68a1e02d3e9a811eaf9ab49961d9d63c>) * build python3-daemon rpm from python-daemon-3k release <https://github.com/jbvsmo/python-daemon/archive/1.5.8.tar.gz> [...]
I find it amusing that the comparison is trying to count *GitHub Stars* for a project which isn't actually hosted on GitHub (and when were those ever a mark of quality in the first place?), but more amazing is that it seems to have ignored that python-daemon-3k was last released in 2014 and its Git repository has seen no new commits since 2016, while python-daemon's last release was a mere 6 months ago. Also the patch referred to there looks like it would apply cleanly to both projects (python-daemon-3k is really just a very outdated fork of python-daemon anyway). Similar patches have been proposed to python-daemon, so maybe helping to address the maintainer's concerns with one of those would be a greater benefit to everyone? https://pagure.io/python-daemon/pull-request/7 https://pagure.io/python-daemon/pull-request/27 -- Jeremy Stanley
If the sole problem here is the /dev/null redirects, then let's just drop the redirects: https://opendev.org/starlingx/utilities/src/branch/master/utilities/logmgmt/... I don't think that should have any impact. -----Original Message----- From: Jeremy Stanley [mailto:fungi@yuggoth.org] Sent: Tuesday, April 28, 2020 10:25 AM To: starlingx-discuss@lists.starlingx.io Subject: Re: [Starlingx-discuss] FW: [starlingx-discuss][python3] four solutions for "Add python3-daemon required by logmgmt" On 2020-04-28 06:44:35 -0700 (-0700), Saul Wold wrote:
I am not sure I understand how option 1 and option 3 are different both are modifying upstream projects and require the StarlingX community to maintain a patched version (ie technical debt).
Is there an option that modifies the logmgmt code directly instead of changing the daemon code? [...]
It might be possible to subclass daemon.runner and shadow the offending methods, rather than forking the entire codebase.
Option 3: import python-daemon-3k [2] python3 condition.
* use "python-daemon-3k" instead of "python-daemon" when the env is python3 * add patch to fix the bug of python-daemon in python3 (refer to pull request #1 from takuti/fix-unbuffered-error <https://github.com/jbvsmo/python-daemon/commit/91567c9e68a1e02d3e9a811eaf9ab49961d9d63c>) * build python3-daemon rpm from python-daemon-3k release <https://github.com/jbvsmo/python-daemon/archive/1.5.8.tar.gz> [...]
I find it amusing that the comparison is trying to count *GitHub Stars* for a project which isn't actually hosted on GitHub (and when were those ever a mark of quality in the first place?), but more amazing is that it seems to have ignored that python-daemon-3k was last released in 2014 and its Git repository has seen no new commits since 2016, while python-daemon's last release was a mere 6 months ago. Also the patch referred to there looks like it would apply cleanly to both projects (python-daemon-3k is really just a very outdated fork of python-daemon anyway). Similar patches have been proposed to python-daemon, so maybe helping to address the maintainer's concerns with one of those would be a greater benefit to everyone? https://pagure.io/python-daemon/pull-request/7 https://pagure.io/python-daemon/pull-request/27 -- Jeremy Stanley
@Penney, Don drop codes (https://opendev.org/starlingx/utilities/src/branch/master/utilities/logmgmt/...) could not solve the problem bringing by python3 upgradation. The root cause is a bug of python-daemon upstream. @Jeremy Stanley thanks for your "subclass daemon.runner " advice which simplifies the Option 2 and makes it become a good option to choose. By the way, the offline patch in Option 1 is refer to https://pagure.io/python-daemon/pull-request/27, in fact . @Wold, Saul the different between Option1 and Option3 is the bug was fixed in master branch of project https://github.com/jbvsmo/python-daemon and we can expect to drop the offline patch when it comes a new release. However I didn't notice the project imported in Option 3 had not updated release for 6 years, so I'll drop option 3. By the way, both Option2 and Option4 are modifying the logmgmt code directly instead of changing the daemon code. In last, we'll try to commit pull-request to python-daemon upstream to fix this bug. But before that we'll implement Option 2 to support logmgmt python3 upgradation. " Option 2: subclass "daemon.runner.DaemonRunner()" as “MyDaemonRunner()” and override methods refer to https://pagure.io/python-daemon/pull-request/27 . 1) using class “MyDaemonRunner()” instead of “daemon.runner.DaemonRunner()” to create daemon runner; 2) build python3-daemon rpm from python-daemon srpm" Thanks Ran -----Original Message----- From: Penney, Don <Don.Penney@windriver.com> Sent: Tuesday, April 28, 2020 10:35 PM To: Jeremy Stanley <fungi@yuggoth.org>; starlingx-discuss@lists.starlingx.io Subject: Re: [Starlingx-discuss] FW: [starlingx-discuss][python3] four solutions for "Add python3-daemon required by logmgmt" If the sole problem here is the /dev/null redirects, then let's just drop the redirects: https://opendev.org/starlingx/utilities/src/branch/master/utilities/logmgmt/... I don't think that should have any impact. -----Original Message----- From: Jeremy Stanley [mailto:fungi@yuggoth.org] Sent: Tuesday, April 28, 2020 10:25 AM To: starlingx-discuss@lists.starlingx.io Subject: Re: [Starlingx-discuss] FW: [starlingx-discuss][python3] four solutions for "Add python3-daemon required by logmgmt" On 2020-04-28 06:44:35 -0700 (-0700), Saul Wold wrote:
I am not sure I understand how option 1 and option 3 are different both are modifying upstream projects and require the StarlingX community to maintain a patched version (ie technical debt).
Is there an option that modifies the logmgmt code directly instead of changing the daemon code? [...]
It might be possible to subclass daemon.runner and shadow the offending methods, rather than forking the entire codebase.
Option 3: import python-daemon-3k [2] python3 condition.
* use "python-daemon-3k" instead of "python-daemon" when the env is python3 * add patch to fix the bug of python-daemon in python3 (refer to pull request #1 from takuti/fix-unbuffered-error <https://github.com/jbvsmo/python-daemon/commit/91567c9e68a1e02d3e9a811eaf9ab49961d9d63c>) * build python3-daemon rpm from python-daemon-3k release <https://github.com/jbvsmo/python-daemon/archive/1.5.8.tar.gz> [...]
I find it amusing that the comparison is trying to count *GitHub Stars* for a project which isn't actually hosted on GitHub (and when were those ever a mark of quality in the first place?), but more amazing is that it seems to have ignored that python-daemon-3k was last released in 2014 and its Git repository has seen no new commits since 2016, while python-daemon's last release was a mere 6 months ago. Also the patch referred to there looks like it would apply cleanly to both projects (python-daemon-3k is really just a very outdated fork of python-daemon anyway). Similar patches have been proposed to python-daemon, so maybe helping to address the maintainer's concerns with one of those would be a greater benefit to everyone? https://pagure.io/python-daemon/pull-request/7 https://pagure.io/python-daemon/pull-request/27 -- Jeremy Stanley _______________________________________________ Starlingx-discuss mailing list Starlingx-discuss@lists.starlingx.io http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss
On 4/28/20 7:25 AM, Jeremy Stanley wrote:
On 2020-04-28 06:44:35 -0700 (-0700), Saul Wold wrote:
I am not sure I understand how option 1 and option 3 are different both are modifying upstream projects and require the StarlingX community to maintain a patched version (ie technical debt).
Is there an option that modifies the logmgmt code directly instead of changing the daemon code? [...]
It might be possible to subclass daemon.runner and shadow the offending methods, rather than forking the entire codebase.
Option 3: import python-daemon-3k [2] python3 condition.
* use "python-daemon-3k" instead of "python-daemon" when the env is python3 * add patch to fix the bug of python-daemon in python3 (refer to pull request #1 from takuti/fix-unbuffered-error <https://github.com/jbvsmo/python-daemon/commit/91567c9e68a1e02d3e9a811eaf9ab49961d9d63c>) * build python3-daemon rpm from python-daemon-3k release <https://github.com/jbvsmo/python-daemon/archive/1.5.8.tar.gz> [...]
I find it amusing that the comparison is trying to count *GitHub Stars* for a project which isn't actually hosted on GitHub (and when were those ever a mark of quality in the first place?), but more amazing is that it seems to have ignored that python-daemon-3k was last released in 2014 and its Git repository has seen no new commits since 2016, while python-daemon's last release was a mere 6 months ago.
Agreed here, python-daemon is at least creating current releases.
Also the patch referred to there looks like it would apply cleanly to both projects (python-daemon-3k is really just a very outdated fork of python-daemon anyway). Similar patches have been proposed to python-daemon, so maybe helping to address the maintainer's concerns with one of those would be a greater benefit to everyone?
BTW, I did try to reach out to Ben Finney (bignose) at his Debian email, but have not gotten a response, if you know of another way to reach out to him could you let us know. Sau!
_______________________________________________ Starlingx-discuss mailing list Starlingx-discuss@lists.starlingx.io http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss
On 2020-04-28 08:00:37 -0700 (-0700), Saul Wold wrote: [...]
BTW, I did try to reach out to Ben Finney (bignose) at his Debian email, but have not gotten a response, if you know of another way to reach out to him could you let us know.
He has some IRC and Matrix contact information listed at https://wiki.debian.org/BenFinney but also maybe just raising a comment on one of the relevant issues or pull requests for https://pagure.io/python-daemon/ would get his attention? -- Jeremy Stanley
participants (4)
-
An, Ran1
-
Jeremy Stanley
-
Penney, Don
-
Saul Wold