GeoNode / geonode-docker

Django base images for GeoNode
Other
9 stars 33 forks source link

Can't rename db instance #15

Closed Spy-Shifty closed 5 years ago

Spy-Shifty commented 5 years ago

When using docker-compose the database instance has to be "db" because of:

def _update_db_connstring():
    user = os.getenv('GEONODE_DATABASE', 'geonode')
    pwd = os.getenv('GEONODE_DATABASE_PASSWORD', 'geonode')
    dbname = os.getenv('GEONODE_DATABASE', 'geonode')
    connstr = 'postgres://{0}:{1}@db:5432/{2}'.format(
        user,
        pwd,
        dbname
    )
    return connstr

def _update_geodb_connstring():
    geouser = os.getenv('GEONODE_GEODATABASE', 'geonode_data')
    geopwd = os.getenv('GEONODE_GEODATABASE_PASSWORD', 'geonode_data')
    geodbname = os.getenv('GEONODE_GEODATABASE', 'geonode_data')
    geoconnstr = 'postgis://{0}:{1}@db:5432/{2}'.format(
        geouser,
        geopwd,
        geodbname
    )
    return geoconnstr

Please change the code to some thing like this:

def _update_db_connstring():
    geoconnstr = os.getenv('GEONODE_DATABASE_URL', '')
    return geoconnstr

def _update_geodb_connstring():
    geoconnstr = os.getenv('GEONODE_GEODATABASE_URL', '')
    return geoconnstr

Where the environment variables are like:

GEONODE_DATABASE_URL=postgres://geonode:geonode@geoserver-db:5432/geonode
GEONODE_GEODATABASE_URL=postgres://geonode_data:geonode_data@geoserver-db:5432/geonode_data
francbartoli commented 5 years ago

@Spy-Shifty the database instance is taking the hostname from the service name declared in the docker-compose file. This is by design for docker-compose networking. We actually don't need to have it dynamically defined because it is used only for internal communication among containers. I'm going to close this.

ingenieroariel commented 5 years ago

Francesco, the snippet of code he linked to has "db" hardcoded. Are you sure there is a parameter to change it?

On Wed, Jul 10, 2019 at 9:31 AM Francesco Bartoli notifications@github.com wrote:

Closed #15 https://github.com/GeoNode/geonode-docker/issues/15.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/GeoNode/geonode-docker/issues/15?email_source=notifications&email_token=AAANNV2QPWJPLVPQHTH4C4DP6XXCTA5CNFSM4HANCJ72YY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOSNR2OZY#event-2472781671, or mute the thread https://github.com/notifications/unsubscribe-auth/AAANNV4Q3JOCQQ3YZVSKOMDP6XXCTANCNFSM4HANCJ7Q .

francbartoli commented 5 years ago

Hi @ingenieroariel no there isn’t but that change doesn’t make sense to me. Why should we make it a variable? What’s the benefit?

ingenieroariel commented 5 years ago

I just checked the actual code, since it is all in tasks.py and that is tied to the current docker implementation I guess it is okay and end users can just override it if they need to.

My main gripe was if it was harcoded in GeoNode itself (the django project). -a

On Wed, Jul 10, 2019 at 10:39 AM Francesco Bartoli notifications@github.com wrote:

Hi @ingenieroariel https://github.com/ingenieroariel no there isn’t but that change doesn’t make sense to me. Why should we make it a variable? What’s the benefit?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GeoNode/geonode-docker/issues/15?email_source=notifications&email_token=AAANNV7CPW5XOM2D2GN7J73P6X7CJA5CNFSM4HANCJ72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZT3ZYI#issuecomment-510115041, or mute the thread https://github.com/notifications/unsubscribe-auth/AAANNV5LQQSDC4KVFWGPXVLP6X7CJANCNFSM4HANCJ7Q .