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

[WIP] django-admin for pulp #630

Closed
wants to merge 3 commits into from
Closed

[WIP] django-admin for pulp #630

wants to merge 3 commits into from

Conversation

alikins
Copy link
Contributor

@alikins alikins commented Apr 3, 2020

Enable django-admin, and include some ModelAdmin's generated with
https://django-extensions.readthedocs.io/en/latest/admin_generator.html

Enabling django-admin in pulpcore makes it easier to use it in plugins.

  • they don't have to jump through hoops to enable it and add it to DJANGO_APPS
  • If the pulpcore models have ModelAdmin classes available, plugins that want to
    provide django-admin setup for dealing with models that use core models, they don't need to
    bundle or duplicate them.

It's been a few weeks since I last updated the ModelAdmin stuff, so it may be slightly out of sync.
It would probably be useful to automate the process of build the admin pages, so things dont drift.

@pulpbot
Copy link
Member

pulpbot commented Apr 3, 2020

WARNING!!! This PR is not attached to an issue. In most cases this is not advisable. Please see our PR docs for more information about how to attach this PR to an issue.

@bmbouter
Copy link
Member

bmbouter commented Apr 9, 2020

I was talking about this at the pulpcore checkin meeting, and some practical concerns that came up in using django-admin with Pulp. We are thinking we should document users not to expect it and why. I created an issue here to document why something so well-known and useful in the Django community is probably not a good fit for Pulp. https://pulp.plan.io/issues/6475

Assuming the DRF browsable API provides most read functionality, can you call out the API gaps pulp has that caused you to want to use django-admin in the first place? In terms of this PR we probably should close it since the current thinking is that it isn't safe with Pulp.

@alikins
Copy link
Contributor Author

alikins commented Apr 9, 2020

For my uses, there tends to be cases where I would find django-admin useful.

  • The DRF browseable API isn't as useful since it is often not exposed directly for automation-hub / galaxy_ng. The hosted cloud.redhat.com automation-hub doesn't expose it all. Granted, neither would any django-admin urls and the automation-hub style deploys are not a normal scenario.

  • With some minimal custom AdminModels, django-admin makes it easier to do searching/filtering than trying to do it via the API. The downside is the django-admin style searching options will likely be different than the search options exposed via the API. (The django-extensions admin generator for an example of a way to automatically created better than default AdminModels)

  • django-admin tends to map more directly to db tables and django orm models than the full REST API. Though both do their fair share of custom serialization and DTO or forms. That's not better or worse, it's just different. Sometimes the db-focused style is more convenient, sometimes not, but can be a useful option.

  • I don't tend to think of the browseable API as being particularly friendly to 'browsing'. Looking at pages of JSON blobs can be hard to grok quickly, especially for lists. It's hard to turn a long page of nested JSON into a mental 'oh, there are 3 Repositories defined'

  • The browseable API doesn't seem to be complete. For example, http://PULPHOST/pulp/api/v3/
    isn't 'hyperlinked' and diving into say /pulp/api/v3/status/ requires entering that URL. Granted, this is probably more of a temporary bug than anything.

@alikins
Copy link
Contributor Author

alikins commented May 18, 2020

I rebased this, and split it into two prs. One for just enabling a /admin url.

And #705 that adds some slightly customized AdminModels.

@bmbouter
Copy link
Member

bmbouter commented Jun 5, 2020

With my RBAC proof of concept it likely will also use django-admin. I hope to have it to a ready-to-discuss state by end of next week. At that time I also want to look at the django admin functionality this provides to see what a "total django admin" user experience would look like.

In other words, I'll review this hopefully in about a week from now. Thanks for submitting.

@alikins
Copy link
Contributor Author

alikins commented Aug 11, 2020

obsoleted by #815 3adeff2

@alikins alikins closed this Aug 11, 2020
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.

None yet

5 participants