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

Move core code out of __init__.py into management/__init__.py #81

Closed
bckohan opened this issue Jun 1, 2024 · 3 comments
Closed

Move core code out of __init__.py into management/__init__.py #81

bckohan opened this issue Jun 1, 2024 · 3 comments
Assignees
Labels
enhancement New feature or request question Further information is requested
Milestone

Comments

@bckohan
Copy link
Owner

bckohan commented Jun 1, 2024

The original intent of having the core code in __init__ was so that imports were simple and in the django_typer namespace. Theres one downside to this which is that this code will be loaded during non-management command bootstraps (i.e. serving mode in production) if django-typer is installed as an app (it will also be loaded regardless if someone runs get_command or call_command from serving code). The memory/time impact of this is probably negligible. click and and typer are small and self contained and all other imports are stdlib stuff thats probably loaded from elsewhere anyway. Nevertheless its probably best to avoid unnecessary runtime imports.

This of course can be avoided by running an app stack without django_typer in serving mode.

At the very least add a section in the documentation including an easy strategy for running a separate app stack when run through manage.py - which is not a big ask of any projects that want to use typer but are also concerned about serving memory footprints that are as small as possible.

Before making a call for version 3 - which would be a breaking change - should quantify the impacts. Its probably small single digit KB memory hit.

@bckohan bckohan added the question Further information is requested label Jun 1, 2024
@bckohan bckohan self-assigned this Jun 1, 2024
@bckohan bckohan added this to the Version 3.0 milestone Jun 1, 2024
@bckohan
Copy link
Owner Author

bckohan commented Jun 4, 2024

Another reason to do this is to avoid the need for lazy translation in the common option helps.

I think the best location would be to put everything thats in django_typer/init.py into django_typer/management/init.py. This would also mirror how you import BaseCommand.

Should probably go ahead and do this, then update all of the docs and import the symbols back into django_typer/init.py for backwards compat so when version 3.0 comes around most code will not be impacted.

@bckohan bckohan added the enhancement New feature or request label Jun 5, 2024
@bckohan bckohan changed the title Move core code out of __init__.py? Move core code out of __init__.py into management/__init__.py Jun 5, 2024
@bckohan
Copy link
Owner Author

bckohan commented Jun 5, 2024

Ok, it makes more sense to do this than not. Until version 3 django_typer/init.py will maintain these imports so as not to break existing code, but after 3 they will be removed. Better to make this change sooner than later and update all the docs to limit the pain later.

bckohan added a commit that referenced this issue Jun 6, 2024
@bckohan
Copy link
Owner Author

bckohan commented Jun 6, 2024

The module footprint increase relative to a minimalist django install seems to be 183 modules (~20%) which is significant. Most apps will be much less because they do things, but still. This change is worth the disruption.

@bckohan bckohan closed this as completed Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

1 participant