Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make kosh easier to interactively debug #132

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Querela
Copy link
Contributor

@Querela Querela commented Jul 17, 2023

This change just refactors and extracts some setup code from kosh.main() to kosh.setup. No other changes.

This allows to call kosh.setup() without starting kosh with the blocking kosh.serve() which hinders interactive debugging.

Example (python shell):

>>> # still need to set the CLI args (to simulate normal startup process with arguments)
>>> import sys
>>> sys.argv[:] = ["__main__", "--config_file", "kosh.ini", "--log_level", "DEBUG"]

>>> # now "start" kosh (parse CLI args, setup internal fields)
>>> import kosh.kosh
>>> app = kosh.kosh.kosh()
>>> app.setup()

>>> # now we can manipulate the kosh instance, e.g.
>>> from kosh.utility.instance import instance
>>> lexicon = instance.lexicons["hoenig"]
>>> from kosh.elastic.search import search
>>> search.entries(lexicon, "lemma_ksh", "aach", "term", 10)
... # ...

--> kosh.setup() can now be called without it starting to serve()

Example (python shell):

>>> # still need to set the CLI args (simulate)
>>> import sys
>>> sys.argv[:] = ["__main__", "--config_file", "kosh.ini", "--log_level", "DEBUG"]

>>> # now "start" kosh
>>> import kosh.kosh
>>> app = kosh.kosh.kosh()
>>> app.setup()

>>> # now we can manipulate the kosh instance, e.g.
>>> from kosh.utility.instance import instance
>>> lexicon = instance.lexicons["hoenig"]
>>> from kosh.elastic.search import search
>>> search.entries(lexicon, "lemma_ksh", "aach", "term", 10)
... # ...
@schlusslicht
Copy link
Member

First of all, I second Your idea. But may I ask for Your thoughts about utilizing and/or trimming down the current execution path if kosh is run as __main__? Given Your example,

sys.argv[:] = ["__main__", "--config_file", "kosh.ini", "--log_level", "DEBUG"]
import kosh.kosh

And the current structure of the entrypoint module,

def main() -> None:
    kosh().main()

if __name__ == "__main__":
    main()

class kosh:
    # [...]

    def main(self) -> None:
        # [...]

it seems to me that, at least, the __main__ => main() => kosh().main() chaining could be trimmed down to __main__ => kosh().main(). But given this issue and the case You are making to ease debugging kosh, I guess the blocking serve() call could be conditionalized upon __name__ == "__main__".

I'm not quite sure yet if and how this could benefit the execution path and therefor debugging the code and I'm also no against merging Your pull request but I my gut tells me there's a greater benefit to be had here. :-)

@Querela
Copy link
Contributor Author

Querela commented Sep 1, 2023

My idea was to mostly keep the same structure but only extract the setup code into its method. As the intended main use-case is running the server, I didn't want to modify too much, so kosh.main() is the entry-point.

The kosh().main() method could have an optional parameter with (serve=True) to allow to opt-out of serving but for debugging I wouldn't even call this function if I have a clean kosh.setup() method.

I believe the global main() function is just for the CLI entry-point (<package>.<module>:method).

kosh = "kosh.kosh:main"

With if __name__ == "__main__": for script usage.

# untested, not sure about path prefixes or not, and whether kosh needs to be installed or not
# run when installed and called from CLI (`python -m kosh` ?)
def main() -> None:
    kosh().main()

# so run as script `./kosh.py` / `python kosh.py`
if __name__ == "__main__":
    main()

As as side note, Flask suggests a __main__.py file as entry-point but this is mostly due to circular imports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants