[sssd PR#5845][opened] sss-analyze: Fix self imports

[stanislavlevin]

   URL: https://github.com/SSSD/sssd/pull/5845
Author: stanislavlevin
 Title: #5845: sss-analyze: Fix self imports
Action: opened

PR body:
"""
- fixed self imports to allow any other Python stuff use `sssd`
  Python package
    
- `sssd` Python package tranformed from namespace to regular one
    
- moved the executable out to sbin directory to split the library and
  actual executable (sbin because of only root can read sssd logs)
    
Resolves: https://github.com/SSSD/sssd/issues/5842
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5845/head:pr5845
git checkout pr5845

[sssd PR#5845][edited] sss-analyze: Fix self imports

[stanislavlevin]

   URL: https://github.com/SSSD/sssd/pull/5845
Author: stanislavlevin
 Title: #5845: sss-analyze: Fix self imports
Action: edited

 Changed field: body
Original value:
"""
- fixed self imports to allow any other Python stuff use `sssd`
  Python package
    
- `sssd` Python package tranformed from namespace to regular one
    
- moved the executable out to sbin directory to split the library and
  actual executable (sbin because of only root can read sssd logs)
    
Resolves: https://github.com/SSSD/sssd/issues/5842
"""

[sssd PR#5845][comment] sss-analyze: Fix self imports

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

pbrezina commented:
"""
Thank you.

>      * moved the executable out to sbin directory to split the library
> and
>        actual executable (sbin because of only root can read sssd logs)
>  
I am not entirely sure this is desirable. The script should be called through 'sssctl
analyze' so I don't think we really want to make it accessible from sbin.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-952814625

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
>  Thank you.
>  
>  > ```
>  > * moved the executable out to sbin directory to split the library and
>  >   actual executable (sbin because of only root can read sssd logs)
>  > ```
>  
>  I am not entirely sure this is desirable. The script should be called through 'sssctl
> analyze' so I don't think we really want to make it accessible from sbin.
>  
if it is only for internal/library usage then it should be in libexec.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-952817770

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
actually, I first checked it as a standalone tool, and only after that I see it is also
called from sssctl.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-952820131

[sssd PR#5845][comment] sss-analyze: Fix self imports

[justin-stephenson]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

justin-stephenson commented:
"""
Thank you for the fixes!

Overall changes LGTM, but make rpms fails with:
~~~
+ /usr/lib/rpm/redhat/brp-mangle-shebangs
mangling shebang in /usr/sbin/sss_debuglevel from /bin/sh to #!/usr/bin/sh
*** ERROR: ambiguous python shebang in /usr/sbin/sss_analyze: #!/usr/bin/env python.
Change it to python3 (or python2) explicitly.
error: Bad exit status from /var/tmp/rpm-tmp.J0MFzZ (%install)
~~~
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-952975624

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
@justin-stephenson, thank you for the review!
Can I see the build log? which distro/version?
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-953004119

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
F34: `make rpms` works for me.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-953051417

[sssd PR#5845][comment] sss-analyze: Fix self imports

[justin-stephenson]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

justin-stephenson commented:
"""
>  @justin-stephenson, thank you for the review! Can I see the build
> log? which distro/version?
>  
Sorry disregard, leftover build settings issue.

Ack from my side.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-953201029

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
@justin-stephenson, @pbrezina, please let me know if something else should be done for
this PR.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-953557202

[sssd PR#5845][comment] sss-analyze: Fix self imports

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

pbrezina commented:
"""
@justin-stephenson Is there any intention to run sss_analyze as a standalone program? Is
it documented anywhere like this?
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-954684239

[sssd PR#5845][comment] sss-analyze: Fix self imports

[justin-stephenson]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

justin-stephenson commented:
"""
>  @justin-stephenson Is there any intention to run sss_analyze as a
> standalone program? Is it documented anywhere like this?
>  
No, I have only been developing it and writing documentation for it to be called with
sssctl analyze.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-954716430

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
Any objections about libexec for sss_analyze?
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-954738855

[sssd PR#5845][comment] sss-analyze: Fix self imports

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

alexey-tikhonov commented:
"""
>  Any objections about libexec for sss_analyze?
>  
@pbrezina , @justin-stephenson ?
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-958090313

[sssd PR#5845][comment] sss-analyze: Fix self imports

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

pbrezina commented:
"""
I'm fine with libexec.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-958777921

[sssd PR#5845][synchronized] sss-analyze: Fix self imports

[stanislavlevin]

   URL: https://github.com/SSSD/sssd/pull/5845
Author: stanislavlevin
 Title: #5845: sss-analyze: Fix self imports
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5845/head:pr5845
git checkout pr5845

[sssd PR#5845][synchronized] sss-analyze: Fix self imports

[stanislavlevin]

   URL: https://github.com/SSSD/sssd/pull/5845
Author: stanislavlevin
 Title: #5845: sss-analyze: Fix self imports
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5845/head:pr5845
git checkout pr5845

[sssd PR#5845][synchronized] sss-analyze: Fix self imports

[stanislavlevin]

   URL: https://github.com/SSSD/sssd/pull/5845
Author: stanislavlevin
 Title: #5845: sss-analyze: Fix self imports
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5845/head:pr5845
git checkout pr5845

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
@pbrezina, thanks, updated.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-958934172

[sssd PR#5845][comment] sss-analyze: Fix self imports

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

pbrezina commented:
"""
Wouldn't it make sense to put everything related to sss_analyze to libexec? We
don't provide any reusable interface at this moment.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-959390049

[sssd PR#5845][comment] sss-analyze: Fix self imports

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

pbrezina commented:
"""
Hi @stanislavlevin, we will do a release soon. What do you think about moving all to
libexec given we don't plan to publish any Python interface for 3rd party? (At least
not at the moment). If you think it is a good idea (I admit we don't have much
experience with Python packaging), could you amend the patch? Thanks.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-963355255

[sssd PR#5845][comment] sss-analyze: Fix self imports

[Dessa]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

Dessa commented:
"""
i'd like to see the --without-python3-bindings case handled too - because currently it
installs files to **/**sssd
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-966582530

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
Hi, @pbrezina. Sorry, I had my vacation.
Given the Python stuff is not planned to be reusable and to not pollute `sssd` package
(Python distribution having name `sssd`) without a good reason the `sssd` stuff can be
moved into libexec. tbh, I wondered when I saw sssd tools in `sssd` distribution. Working
on fix.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-968636231

[sssd PR#5845][synchronized] sss-analyze: Fix self imports

[stanislavlevin]

   URL: https://github.com/SSSD/sssd/pull/5845
Author: stanislavlevin
 Title: #5845: sss-analyze: Fix self imports
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5845/head:pr5845
git checkout pr5845

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
>  i'd like to see the --without-python3-bindings case handled too -
> because currently it installs files to **/**sssd
>  
it's no longer installed into system site packages.
In my opinion `--without-python-bindings` is irrelevant to this script since bindings do
exactly what it says.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-969099415

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
@pbrezina, updated.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-969100053

[sssd PR#5845][edited] sss-analyze: Fix self imports

[stanislavlevin]

   URL: https://github.com/SSSD/sssd/pull/5845
Author: stanislavlevin
 Title: #5845: sss-analyze: Fix self imports
Action: edited

 Changed field: body
Original value:
"""
- fixed self imports to allow any other Python stuff use `sssd`
  Python package
    
- transformed `sssd` Python package from namespace to regular one
    
- moved the executable out to libexec directory to split library and
  actual executable (sss_analyze is not intended to be a standalone tool)
    
Resolves: https://github.com/SSSD/sssd/issues/5842
"""

[sssd PR#5845][comment] sss-analyze: Fix self imports

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

pbrezina commented:
"""
Kind of related: https://github.com/SSSD/sssd/pull/5886
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-976703369

[sssd PR#5845][comment] sss-analyze: Fix self imports

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

pbrezina commented:
"""
Thank you for the update. In general, I agree with the patches. But I think the proper
location require a team discussion about our future plans with python tools so we
don't end up moving the files again and again.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-976705426

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
>  Thank you for the update. In general, I agree with the patches. But I
> think the proper location require a team discussion about our future plans with python
> tools so we don't end up moving the files again and again.
>  
@pbrezina, sure. Please, ping me once the team has any solution.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-976765012

[sssd PR#5845][comment] sss-analyze: Fix self imports

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

pbrezina commented:
"""
Hi, our plan is to convert the whole sssctl command into Python in near future so we can
extend the tool more quickly. Since it will be all Python, it should be placed in the
Python directories. Given this plan, it will be better to keep the analyzer there as well
so we don't move it again and again.

I'm sorry that we wasted your time. Can you please just submit the original PR which
just fixes the self imports? Thank you.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-984548558

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
@pbrezina, hi, no problem.

The original patch( 1e4d2ebc4ad0d69afa00ab3d2bb481a8f5ddef80) moved `sss_analyze` into
sbindir, is it ok?
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-984562283

[sssd PR#5845][comment] sss-analyze: Fix self imports

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

pbrezina commented:
"""
I wouldn't move it into sbin dir either.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-985529530

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
@pbrezina, you may want to read:
https://ocs.python.org/3/library/sysconfig.html#installation-paths
>  Each scheme is itself composed of a series of paths and each path has
> a unique identifier. Python currently uses eight paths:
>  
stdlib: directory containing the standard Python library files that are not
platform-specific.
platstdlib: directory containing the standard Python library files that are
platform-specific.
platlib: directory for site-specific, platform-specific files.
purelib: directory for site-specific, non-platform-specific files.
include: directory for non-platform-specific header files.
platinclude: directory for platform-specific header files.
scripts: directory for script files.
data: directory for data files.

`sss_analyze.py` is the combo of module and script. Again, I suggest moving it either to
sbin or libexec. If you don't want to expose the script to end-users, then it can be
hidden under libexec. Later, on refactoring sssctl the no longer script can be safely
removed.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-986736988

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
@pbrezina, you may want to read:
https://docs.python.org/3/library/sysconfig.html#installation-paths
>  Each scheme is itself composed of a series of paths and each path has
> a unique identifier. Python currently uses eight paths:
>  
stdlib: directory containing the standard Python library files that are not
platform-specific.
platstdlib: directory containing the standard Python library files that are
platform-specific.
platlib: directory for site-specific, platform-specific files.
purelib: directory for site-specific, non-platform-specific files.
include: directory for non-platform-specific header files.
platinclude: directory for platform-specific header files.
scripts: directory for script files.
data: directory for data files.

`sss_analyze.py` is the combo of module and script. Again, I suggest moving it either to
sbin or libexec. If you don't want to expose the script to end-users, then it can be
hidden under libexec. Later, on refactoring sssctl the no longer script can be safely
removed.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-986736988

[sssd PR#5845][comment] sss-analyze: Fix self imports

[justin-stephenson]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

justin-stephenson commented:
"""
Overall i'm fine to move the files into `libexec/sssd` but as we suspect there will be
more changes to `sss_analyze`/`sssctl` python packaging in the near future, in my opinion
it makes sense to leave the files as is (as we just released the analyzer installing into
`site-packages`) unless there is a compelling enough reason for the move. 
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-990036563

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
Is FHS enough reason?
https://refspecs.linuxfoundation.org/FHS_3.0/fhs-3.0.html#usrlibLibraries...
>  4.6. /usr/lib : Libraries for programming and packages
>  
4.6.1. Purpose
/usr/lib includes object files and libraries. [21] On some systems, it may also include
internal binaries that are not intended to be executed directly by users or shell scripts.
[22]
> Applications may use a single subdirectory under /usr/lib. If an application uses a
> subdirectory, all architecture-dependent data exclusively used by the application must be
> placed within that subdirectory. [23]
>
> 4.7. /usr/libexec : Binaries run by other programs (optional)
>  
4.7.1. Purpose
/usr/libexec includes internal binaries that are not intended to be executed directly by
users or shell scripts. Applications may use a single subdirectory under /usr/libexec.
> Applications which use /usr/libexec in this way must not also use /usr/lib to store
> internal binaries, though they may use /usr/lib for the other purposes documented here.
>
> Rationale
>  
Some previous versions of this document did not support /usr/libexec, despite it
being standard practice in a number of environments. [26] To accomodate this restriction,
it became common practice to use /usr/lib instead. Either practice is now acceptable, but
each application must choose one way or the other to organize itself.

It *was* and *is* an error to package an executable into Python's site-packages.
I'm trying to understand your objections to this, but can't see, sorry.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-990723195

[sssd PR#5845][comment] sss-analyze: Fix self imports

[justin-stephenson]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

justin-stephenson commented:
"""
Okay, then ack to moving the files to libexec. Sorry for any confusion.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-991022434

[sssd PR#5845][comment] sss-analyze: Fix self imports

[justin-stephenson]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

justin-stephenson commented:
"""
Okay, then ack to moving the files to libexec. Sorry for any confusion.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-991022434

[sssd PR#5845][synchronized] sss-analyze: Fix self imports

[stanislavlevin]

   URL: https://github.com/SSSD/sssd/pull/5845
Author: stanislavlevin
 Title: #5845: sss-analyze: Fix self imports
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5845/head:pr5845
git checkout pr5845

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
@justin-stephenson, how can I interpret the deletion of your comment?
ack or reject or I don't know? :-)

Rebased (hope finally).
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-992326256

[sssd PR#5845][comment] sss-analyze: Fix self imports

[justin-stephenson]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

justin-stephenson commented:
"""
>  @justin-stephenson, how can I interpret the deletion of your comment?
> ack or reject or I don't know? :-)
>  
>  Rebased (hope finally).
>  
We came to a consensus on approving the move of sss_analyze to libexec, now I see
`sss_analyze` is installed there but it is also still being installed into
`site-packages/sssd`. Would you mind fixing this? That should be the last thing needed.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-992714761

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
@justin-stephenson, `sss_analyze.py` is the module lived in site-packages, while
`sss_analyze` is just a wrapper over that module.
Do you want to completely drop `sss_analyze.py` module?
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-993374522

[sssd PR#5845][comment] sss-analyze: Fix self imports

[justin-stephenson]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

justin-stephenson commented:
"""
>  @justin-stephenson, `sss_analyze.py` is the module lived in
> site-packages, while `sss_analyze` is just a wrapper over that module. Do you want to
> completely drop `sss_analyze.py` module?
>  
I see now the addition of the new wrapper, let's leave as is for now. Ack to these
changes.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-993530470

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
@justin-stephenson, thank you for your patience :)
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-993536678

[sssd PR#5845][comment] sss-analyze: Fix self imports

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

alexey-tikhonov commented:
"""
@justin-stephenson, could you please add "Accepted" label?
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-994729902

[sssd PR#5845][+Accepted] sss-analyze: Fix self imports

[justin-stephenson]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

Label: +Accepted

[sssd PR#5845][comment] sss-analyze: Fix self imports

[justin-stephenson]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

justin-stephenson commented:
"""
>  @justin-stephenson, thank you for your patience :)
>  
Thank you!
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-994814674

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
@justin-stephenson, please, be aware that this PR and #5913 will cause a merge conflict.
I suggest merging #5913 first and I can rebase this one then.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-994842916

[sssd PR#5845][comment] sss-analyze: Fix self imports

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

alexey-tikhonov commented:
"""
>  @justin-stephenson, please, be aware that this PR and #5913 will
> cause a merge conflict. I suggest merging #5913 first and I can rebase this one then.
>  
@stanislavlevin, #5913 was merged.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-995784148

[sssd PR#5845][synchronized] sss-analyze: Fix self imports

[stanislavlevin]

   URL: https://github.com/SSSD/sssd/pull/5845
Author: stanislavlevin
 Title: #5845: sss-analyze: Fix self imports
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5845/head:pr5845
git checkout pr5845

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
@justin-stephenson, there is the circular import on the recent change exposed with this
PR:
```
[root@master1 /]# sssctl analyze
NOTE: Tevent chain ID support missing, request analysis will be limited.
It is recommended to use the --logdir option against tevent chain ID supported SSSD
logs.
Traceback (most recent call last):
  File "/usr/libexec/sssd/sss_analyze", line 3, in <module>
    from sssd import sss_analyze
  File "/usr/lib/python3/site-packages/sssd/sss_analyze.py", line 3, in
<module>
    from sssd.modules import request
  File "/usr/lib/python3/site-packages/sssd/modules/request.py", line 6, in
<module>
    from sssd.sss_analyze import SubparsersAction
ImportError: cannot import name 'SubparsersAction' from partially initialized
module 'sssd.sss_analyze' (most likely due to a circular import)
(/usr/lib/python3/site-packages/sssd/sss_analyze.py)
Command '/usr/libexec/sssd/sss_analyze' failed with [1]
```
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-995998297

[sssd PR#5845][synchronized] sss-analyze: Fix self imports

[stanislavlevin]

   URL: https://github.com/SSSD/sssd/pull/5845
Author: stanislavlevin
 Title: #5845: sss-analyze: Fix self imports
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5845/head:pr5845
git checkout pr5845

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
- added a workaround for this, but the sources should be reorganized.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-996009674

[sssd PR#5845][comment] sss-analyze: Fix self imports

[justin-stephenson]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

justin-stephenson commented:
"""
>  @justin-stephenson, there is the circular import on the recent change
> exposed with this PR:
>  
>  ```
>  [root@master1 /]# sssctl analyze
>  NOTE: Tevent chain ID support missing, request analysis will be limited.
>  It is recommended to use the --logdir option against tevent chain ID supported SSSD
> logs.
>  Traceback (most recent call last):
>    File "/usr/libexec/sssd/sss_analyze", line 3, in <module>
>      from sssd import sss_analyze
>    File "/usr/lib/python3/site-packages/sssd/sss_analyze.py", line 3, in
> <module>
>      from sssd.modules import request
>    File "/usr/lib/python3/site-packages/sssd/modules/request.py", line 6, in
> <module>
>      from sssd.sss_analyze import SubparsersAction
>  ImportError: cannot import name 'SubparsersAction' from partially initialized
> module 'sssd.sss_analyze' (most likely due to a circular import)
> (/usr/lib/python3/site-packages/sssd/sss_analyze.py)
>  Command '/usr/libexec/sssd/sss_analyze' failed with [1]
>  ```
>  
I can fix this properly if I can push to your PR here.

class SubparsersAction and class Option can be moved to a separate file(like parser.py),
the Analyzer() instance can be passed as an argument to RequestAnalyzer setup_args()
therefore we don't need to import Analyzer in modules/request
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-996048609

[sssd PR#5845][comment] sss-analyze: Fix self imports

[justin-stephenson]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

justin-stephenson commented:
"""
>  * added a workaround for this, but the sources should be
> reorganized.
>  
HI @stanislavlevin we are nearing an upstream release deadline, and we have an outstanding
PR https://github.com/SSSD/sssd/pull/5863 which also may be affected by this PR. I would
like to cherry pick your patch here and add it to my PR 5863 if it is okay for you. 

I will amend your patch to avoid the circular import workaround as described above, but
will leave you as the original author.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-996056361

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
@justin-stephenson,
- I can drop that workaround part, and then you could open another PR
- I can cherry your commit
- you can cherry or push (if you have corresponding access rights for this repo)
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-996098309

[sssd PR#5845][comment] sss-analyze: Fix self imports

[stanislavlevin]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

stanislavlevin commented:
"""
superseded by https://github.com/SSSD/sssd/pull/5863
"""

See the full comment at https://github.com/SSSD/sssd/pull/5845#issuecomment-996101027

[sssd PR#5845][closed] sss-analyze: Fix self imports

[stanislavlevin]

   URL: https://github.com/SSSD/sssd/pull/5845
Author: stanislavlevin
 Title: #5845: sss-analyze: Fix self imports
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5845/head:pr5845
git checkout pr5845

[sssd PR#5845][-Accepted] sss-analyze: Fix self imports

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

Label: -Accepted

[sssd PR#5845][+superseded] sss-analyze: Fix self imports

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5845
Title: #5845: sss-analyze: Fix self imports

Label: +superseded