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

getting error in admin url #49

Open
shivshankarkeshari opened this issue Sep 7, 2020 · 7 comments
Open

getting error in admin url #49

shivshankarkeshari opened this issue Sep 7, 2020 · 7 comments

Comments

@shivshankarkeshari
Copy link

django.contrib.admin.sites.AlreadyRegistered: The model Consumer is already registered with 'sso_server.ConsumerAdmin'.

@FinalAngel
Copy link
Member

@shivshankarkeshari can you provide additional information on how to reproduce the issue? What Django version you are using etc?

@Rastopapola
Copy link
Contributor

@FinalAngel

I'm running into the same issue, working with Django 3.1.

In my case the issue could be reproduced by adding the django_filters package into my project. Due to simplicity I imported it just like

import django_filters as filters

I was using this way of import in two separate XyzFilter classes, which provided some filtering functionality to my tables. As long as I only had the first XyzFilter imported in some other file, like the views.py, everything was fine. But as soon as I imported the second XyzFilter in another file, the error showed up.

I'm a little confused, since if the way of importing would cause the error, it should not show your consumerAdmin as reason for this.

For me, I fixed it by changing the imports back to

import django_filters

and using them as they are.

@Rastopapola
Copy link
Contributor

@FinalAngel

I found a way to reproduce this:
Add a new path into your app's urls.py just like path('app', app_new_route, name='app-new-route'), where the method app_new_route from your views.py is not imported yet. This may happen if one get's distracted from work and forgets to import or even create it.

However on a re-run of the django runserver command, the initialization of the sso server object inside the main urls.py, -however- triggers this error, which is highly confusing for me as well.

I suggest the following change inside of server.py:

class Server(object):
...
    def register_admin(self):
        try: 
            admin.site.register(Consumer, self.client_admin)
        except AlreadyRegistered:
            # Since this error only occurs if the admin is -idk why- already registered, you might catch this one and continue
             pass

@FinalAngel
Copy link
Member

thanks for the additional info @Rastopapola, would be awesome to have a PR addressing the issue :)

@Rastopapola
Copy link
Contributor

Rastopapola commented Nov 4, 2020

@FinalAngel
No problem, but - on a second thought - I'm not sure whether this is a good solution, simply to try-catch this error.

Since I stumbled on this one again, I'm pretty sure now this is a result of a general missing import statement.
Let's say we have

class A(baseA):
    _a_member = package.Object()

where package has not been imported yet, the import error, that should follow on this results in an AlreadyRegistered error somehow. One should therefore always make sure that every package is imported everywhere - which should be the case normally - but during development some copy-pasting from one file to another may lead to this behaviour, which can be repaired by simply adding the missing import statements

import package 

class A(baseA):
    _a_member = package.Object()

To be honest: I'm absolutely not sure why this error is even thrown at this point... One would expect an ImportError or some kind of this due to the 'unknown' `package declaration.

It's just an idea but maybe the runserver detects the ImportError on a first try and then re-runs some parts, like loading the urls.py again, which results in another try of registering the sso_server's consumer model admin, which results in the AlreadyRegistered error, which simply drops the original ImportError.

Please let me know if I'm sounding strangely confused or this does make sense.

If so, simply try-catching would maybe not the best way in here. Maybe you could refactor the Server class using the singleton pattern such that only one instance of Server can exist and therefore the constructor is only called once?

@Rastopapola
Copy link
Contributor

@FinalAngel I just forked your project and implemented the Singleton pattern. Tests are running fine and when I include my local version of simple-sso, I'm now seeing the correct NameError that is supposed to be there on the import situation described above. As soon as I switch back to your corrent stable release, the missing import leads to the AlreadyRegistered error again. Looks fine!

I will create a PullRequest tomorrow. You may take a look on my changes and by this, I would be very happy to contribute to your project.

@Rastopapola
Copy link
Contributor

Pull Request #52 has been created

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

No branches or pull requests

3 participants