updateadmin tasks should only run on first start

[t-book]

From my understanding: https://github.com/GeoNode/geonode-project/blob/master/entrypoint.sh#L61 should only run on initial start.

For completes this error from the fixture keeps looping the django container on startup

db4dai_geonode | 2020-06-02 08:49:18.924 UTC [48] STATEMENT: INSERT INTO "people_profile" ("id", "password", "last_login", "is_superuser", "username", "first_name", "last_name", "email", "is_staff", "is_active", "date_joined", "organization", "profile", "position", "voice", "fax", "delivery", "city", "area", "zipcode", "country", "language", "timezone") VALUES (1000, 'pbkdf2_sha256$30000$rjuGt0Obn8on$cxF75frIOSaitNklLZ0IJ/VonUW0fwEFVF96o0M+lGc=', '2011-06-09T15:45:34+00:00'::timestamptz, true, 'admin', '', '', '[email protected]', true, true, '2011-06-09T1

[afabiani]

@t-book yes makes sense. The problem is that if you change the admin and geoserver admin passwords or the oauth2 keys on .env, you will need to force reinit at least.

[t-book]

@afabiani I see. updateadmin also takes care of resetting the password: _prepare_admin_fixture(os.environ.get('ADMIN_PASSWORD',

. In that case, maybe it would be conceivable to separate the initial admin fixture creation and the admin step?

(Background: I've migrated a 2.8 db to docker gn3 but got a key error as the admin already existed. Running updateadmin only on init fixed it)

[afabiani]

@t-book yes, it could be an improvement.

However, for the time being, I guess your proposal is better. I'm going to accept your PR. Up to you to merge it or not. The commit should be ported to 3.0 and 3.x branches also imho.

[t-book]

thanks @afabiani After your first comment I saw that updateadmin does more than an initial commit. I think we should not merge my current PR, I will try to come up with a better solution as soon as there is time. Thanks for your review!

[t-book]

in addition

to separate the initial admin fixture creation and the adminupdate step

this is what I would suggest wit a new PR.

[afabiani]

yup, sounds good.

[t-book]

@afabiani a second look ... From what I understand updateadmin() should care for both, create and update the admin profile?

If this is correct I think we do not need it for creating the profile as this should already be covered by sample_admin fixture .

---

For preventing the key_error if the profile already exists we could use the profile model for updating:

@task
def updateadmin():
    print("***********************update admin details**************************")
    import django;
    django.setup();
    from django.contrib.auth import get_user_model;
    User = get_user_model();
    try:
        u = User.objects.get(username="admin");
        u.set_password(os.environ.get('ADMIN_PASSWORD', 'admin'));
        u.email = os.environ.get('ADMIN_EMAIL', '[email protected]');
        u.save();
    except:
        print("No admin user found to update")

---

Having the updateadmin() function in the init if block would lead to the following which might feel like expected behavior

• the user can change admin password and email in .env which is updated on up --build django
• in reverse, the user will not lose a set password on container restart.
• using the profile model over fixtures gives freedom to run it independently, by some other logic or controlled by a new variable UPDATE_ADMIN_PASSWORD = False
• _prepare_admin_fixture would get obsolete

---

I'm unsure if directly accessing the Profile model in tasks.py is against best practice further if I do oversee other things.

Your short feedback is much appreciated (without any hurry)

[t-book]

Unfortunately my suggested fix does not fully solve this issue as still, the fixture could run: https://github.com/GeoNode/geonode-project/blob/master/entrypoint.sh#L46

I think this is the case as after a migration the lockfile does exists and the if statement which checks by or gets true. I do have to check what's the purpose of the lockfile.