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

Second pass at nautobot.apps exports #4459

Closed
lampwins opened this issue Sep 15, 2023 · 4 comments · Fixed by #4533
Closed

Second pass at nautobot.apps exports #4459

lampwins opened this issue Sep 15, 2023 · 4 comments · Fixed by #4533
Assignees
Labels
emergent Unplanned work that is brought into a sprint after it's started.

Comments

@lampwins
Copy link
Member

As a Patti the Plugin Developer, I want the most relevant items from the core codebase to be exported for official use in the nautobot.apps namespace, so I know I am using a blessed interface and so that my imports are consolidated.

I will know this is complete when the core team takes a fresh pass over the core code base to decern items that should reasonably be exported to the apps namespace. The goal will be to export the broadest set of features, leaving only those things that the team has a strong, justifiable reason for not exporting, out of the apps namespace. Obviously if an item is positively known to be currently used by an app, it should be included, but the point of this story is to use good judgement to include things that are either not positively known, or that may reasonably be used in the future.

The app developer and migration documentation should also be updated to clarify the state and purpose of the apps namespace.

There has been some discussion about including models in the namespace and it was not entirely clear if that would pose challenges with FKs or migrations. Unless we feel confident at this time that those issues are resolved, models will be the primary exception to the namespace exports and the docs will clearly spell this out. Model dependent items, such as Choice classes will remain with models and my preference for the docs would be to push app developers to import those items from the model's module to simplify things for app developers.

@lampwins
Copy link
Member Author

@bryanculver
Copy link
Member

As discussed in standup:

Look through Core and Extras and export all functions and classes unless there is a significant reason it should not be exported in apps (ex: Models).

@lampwins lampwins added the emergent Unplanned work that is brought into a sprint after it's started. label Sep 15, 2023
@lampwins lampwins changed the title Second pass as nautobot.apps exports Second pass at nautobot.apps exports Sep 15, 2023
@gsnider2195 gsnider2195 self-assigned this Sep 19, 2023
@gsnider2195
Copy link
Contributor

gsnider2195 commented Sep 22, 2023

After the first pass through core/extras I'm seeing this warning on Nautobot startup:

nautobot-nautobot-1       | 16:25:31.292 WARNING nautobot.core.graphql.utils utils.py        get_filtering_args_from_filterset() :
nautobot-nautobot-1       |   Filter "_custom_field_data" on NamespaceFilterSet is not GraphQL safe, and will be omitted

We'll need to keep in mind that this may be changing the order that modules are being loaded which may be the cause of the above error.

edit:

This was a result of importing nautobot.core.graphql.schema_init

@itdependsnetworks
Copy link
Contributor

itdependsnetworks commented Sep 22, 2023

In theory nothing is calling nautobot.apps EXCEPT a plugin, so the key imports (e.g. ConstanceConfigItem, NautobotAppConfig) should ensure the application is ready before importing anything else or simply ensure the app is ready (maybe nautobot_database_ready?) before any other import in the nautobot.apps directory.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
emergent Unplanned work that is brought into a sprint after it's started.
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants