Skip to content

remove user app from auto included apps#1346

Closed
sinisaos wants to merge 3 commits intopiccolo-orm:masterfrom
sinisaos:cli_changes
Closed

remove user app from auto included apps#1346
sinisaos wants to merge 3 commits intopiccolo-orm:masterfrom
sinisaos:cli_changes

Conversation

@sinisaos
Copy link
Member

@sinisaos sinisaos commented Feb 10, 2026

Related to #1344

@sinisaos sinisaos requested a review from dantownsend February 11, 2026 05:01
@sinisaos
Copy link
Member Author

@dantownsend Have you considered merging this? This small change will allow the user to use a custom user app without interfering with the Piccolo user app. The documentation already states that the user app is optional and if we want use a user app, we need to register it in the AppRegistry. I have also updated the ASGI template so that the user app is included by default. Thanks.

@dantownsend
Copy link
Member

I'm not sure. In terms of the docs, I think you're right - the current behaviour does contradict the docs. I think for new projects it would be OK to not register the user app by default. I just wonder with existing projects if the user app is no longer registered by default if that will have any negative impacts.

Piccolo Admin has the user app in migration_dependencies. I'm not sure if that auto registers the user app. I would need to experiment with it.

I think if someone registered their own user app instead, it might cause problems because they could then never use Piccolo Admin.

@sinisaos
Copy link
Member Author

I'm not sure. In terms of the docs, I think you're right - the current behaviour does contradict the docs. I think for new projects it would be OK to not register the user app by default. I just wonder with existing projects if the user app is no longer registered by default if that will have any negative impacts.

Piccolo Admin has the user app in migration_dependencies. I'm not sure if that auto registers the user app. I would need to experiment with it.

I understand your concern about backward compatibility. I just tried it with some old project and the migrations work because I have registered the Piccolo Admin app. The only thing that doesn't work is creating users because the User app is not registered by default. After registering the User app in piccolo_conf.py everything works as expected.

I think if someone registered their own user app instead, it might cause problems because they could then never use Piccolo Admin.

I think a user with a custom user application that doesn't use BaseUser or a BaseUser subclass is aware of this, because Piccolo Admin depends on session auth which depends on the BaseUser and SessionBase tables and that stated in docs.

@sinisaos
Copy link
Member Author

sinisaos commented Mar 4, 2026

@dantownsend Have you decided whether to make the user app optional or not? If not, I would close this PR. The documentation should also be changed to move the user app from optional includes.

@sinisaos sinisaos closed this Mar 5, 2026
@sinisaos sinisaos deleted the cli_changes branch March 5, 2026 05:14
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.

Piccolo CLI ignores command_name option Piccolo CLI registers user app automatically

2 participants