Cleanup of messed settings and urls

[cezio]

There was a bit of mess in master branch:

• project_name.settings were duplicating whole geonode.settings,
• additionally, project_name.local_settings were added, they didn't include sibling settings, just geonode.settings.
• urls.py were duplicated from geonode.urls in whole.
• celeryapp was added (which is rare case of need), would mask geonode's celery tasks.

This PR cleans it:

• removed local_settings.py from repo, but added local_settings.py.sample, so users can use it for deployment,
• streamlined settings.py, so they customize only actually relevant settings by default,
• removed celeryapp.py.
• updated documentation about settings chaining.

[afabiani]

@simod @jj0hns0n @pjdufour @capooti this PR wants to clean-up settings/loca_settings mess on geonode-project master.

It basically gets rid of duplicated settings from geonode and ships with a minimal set of settings (getting rid also of {{project}}.local_settings which are not needed anymore).

Don't know exactly how this stuff has been messed up, but now should be clean again.

Can you please review and merge?

[jj0hns0n]

@kalxas you are the one who should really look at this.

[capooti]

thanks for taking care of this @cezio.
It is great to have more documentation and less entropy here finally :)
One small note: I think we are discouraging the local_settings.py approach vs the use of environment variables.
Thanks again!

[pjdufour]

I'll defer to Angelos and Jeff on this PR. However, I do want to add that, NOT having "geonode.settings import *" in <project name>.setting was by design and not an error, so to avoid circular imports, etc. Didn't we just have a thread about this last week or something? Please consider those comments. Patrick On Jun 30, 2017 1:47 PM, "Paolo Corti" <notifications@github.com> wrote: thanks for taking care of this @cezio <https://github.com/cezio>. It is great to have more documentation and less entropy here finally :) One small note: I think we are discouraging the local_settings.py approach vs the use of environment variables. Thanks again! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#21 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABXyLX-GRzZXLZ4Pu6jx2iuv4YqErmasks5sJTTMgaJpZM4OKvgx> .

[ingenieroariel]

Paolo is right, the mess on this branch was just the addition of a local settings file that was not there in the first iterations and needlessly imports geonode.settings For things to be more straightforward in the future what we need is less reliance on geonode.settings and eventually drop them favoring people just using Geonode projects. Geonode is both a bunch of apps and a sample Django project, that sample project we ship with should be replaced with an actual template django project so that cpnfiguring it and replacing it is easier. Maybe we should have a dev call about it?

…

On Jun 30, 2017, at 2:10 PM, Patrick Dufour ***@***.***> wrote: I'll defer to Angelos and Jeff on this PR. However, I do want to add that, NOT having "geonode.settings import *" in <project name>.setting was by design and not an error, so to avoid circular imports, etc. Didn't we just have a thread about this last week or something? Please consider those comments. Patrick On Jun 30, 2017 1:47 PM, "Paolo Corti" ***@***.***> wrote: thanks for taking care of this @cezio <https://github.com/cezio>. It is great to have more documentation and less entropy here finally :) One small note: I think we are discouraging the local_settings.py approach vs the use of environment variables. Thanks again! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#21 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABXyLX-GRzZXLZ4Pu6jx2iuv4YqErmasks5sJTTMgaJpZM4OKvgx> . — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[simod]

Yes Ariel. Let's have a meeting in Boston. :) with a call for who cannot attend. Il giorno dom 2 lug 2017 alle 04:13 Ariel Núñez <notifications@github.com> ha scritto:

Paolo is right, the mess on this branch was just the addition of a local settings file that was not there in the first iterations and needlessly imports geonode.settings For things to be more straightforward in the future what we need is less reliance on geonode.settings and eventually drop them favoring people just using Geonode projects. Geonode is both a bunch of apps and a sample Django project, that sample project we ship with should be replaced with an actual template django project so that cpnfiguring it and replacing it is easier. Maybe we should have a dev call about it? > On Jun 30, 2017, at 2:10 PM, Patrick Dufour ***@***.***> wrote: > > I'll defer to Angelos and Jeff on this PR. However, I do want to add that, > NOT having "geonode.settings import *" in <project name>.setting was by > design and not an error, so to avoid circular imports, etc. Didn't we just > have a thread about this last week or something? > > Please consider those comments. > > Patrick > > > On Jun 30, 2017 1:47 PM, "Paolo Corti" ***@***.***> wrote: > > thanks for taking care of this @cezio <https://github.com/cezio>. > It is great to have more documentation and less entropy here finally :) > One small note: I think we are discouraging the local_settings.py approach > vs the use of environment variables. > Thanks again! > > — > You are receiving this because you were mentioned. > > Reply to this email directly, view it on GitHub > < #21 (comment) >, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/ABXyLX-GRzZXLZ4Pu6jx2iuv4YqErmasks5sJTTMgaJpZM4OKvgx > > . > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub, or mute the thread. > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#21 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAvkdHGgbet7rKU-c2ePV-TJ_uHHcI4iks5sJvyzgaJpZM4OKvgx> .

-- Simone

[afabiani]

@pjdufour this PR does what you mentioned (as far as I understood).

Now we have only minimal {{project}}.settings which inherits from geonode.settings

Basically this is a basic, clean structure for {{project}}. It is always possible to have our own {{project}}.local_settings (even if I would suggest to take just one settings}. It is matter of changing the {{project}}.wsgi reference.

[cezio]

NOT having "geonode.settings import *" in .setting was by
design and not an error, so to avoid circular imports, etc. Didn't we just
have a thread about this last week or something?

I'm not aware of such discussion. Could you point me to some archives?

But anyway, approach introduced in master seems to be wrong for several reasons:

• this is code duplication of core element, which is bad in itself. Geonode's settings contain many entries that are geonode-specific, and usually they are good defaults (unless one knows what's doing, no point to change it). The same goes to urls. This stuff is usually too sensitive to change whole file 'just because'.
• I believe most common usage scenario for geonode-project is to have customized html templates, and introduce additional functionalities. In this case, there's no point to duplicate good defaults to geonode-project, only things that are really needs to be changed to have deployment working for user without much of fuss. Bigger customizations require more work, geonode-project won't be helpful then anyway.
• It is common practice to have base settings and deployment-specific settings that import base and change just few things. Using geonode.settings as a base is also more future-proof than having settings duplicated to geonode_project.settings and using that as a base. Geonode's settings change between versions, user would change geonode's required version, ending up with old, potentially broken settings. No need to make things harder than they should be, really.
• Apart from practical inconvenience of specifying env variables to run django app, using environment variables in settings is not very wise, when you operate with complex data structures. With env variable you can pass a string, but not dictionary or a list. I don't think things like

  geonode-project/project_name/settings.py

  Line 363 in 791f791

  INSTALLED_APPS = os.getenv('INSTALLED_APPS',_DEFAULT_INSTALLED_APPS)

  would ever work with env var. Also, from devops perspective: Django provides configuration mechanism already with a separate settings file. No need to reinvent the wheel here with os.environ.

[ingenieroariel]

Cezary, Your understanding of the situation is the same I had several years ago when we introduced local_settings.py and when we just imported from geonode.settings. My opinion has changed after having to maintain several older GeoNoe sites and realizing everytime that it is better to have two things: 1. Snapshot dependencies and settings changes. Having a complete settings file in the downstream project achieves this. 2. It is easier to start with a fresh django project and migrate data, style and custom apps instead of trying to upgrade GeoNode versions on an old project. Pretending to upgrade a Django project to a new GeoNode version relying on whatever is upstream is a recipe for disaster, in the downstream project you don't really know what apps you have installed and what your database looks like and you have less say over your own project. Django provides a built in way to create projects and come with templates, it is called template projects and is based on what the Pinax project contributors learned when trying to implement complex Django projects. The fact we have geonode.settings at all is a mistake in my opinion, that is what causes the problems we are trying to solve and only exists because Django did not have template projects back then. I sincerely believe our approach should be like this: http://pinaxproject.com/pinax/pinax_starter_projects/ In that repo, every branch is a different start project, and you use git in order to keep them in sync. We should not expect projects to just upgrade GeoNode versions blindingly but rather migrate their data, users and media to a fresh custom project. They can use git tools to create a diff and apply it manually on a new starter project. In summary, I bleieve we should prepare the project to delete geonode.settings eventually and replace that workflow with: sudo apt-get install geonode geonode start demo mygeonode cd mygeonode ./deploy (using ansible, docker, etc) Let's have a call to discuss this with you and the rest of the team, I believe it is very important to the project's future that we get to a shared vision, I am willing to be convinced that the old way is better but I wanted to clarify that the snapshotting of the full settings is not an oversight.

…

-a PS: The conversation about env vars is orthogonal, it is useful for people deploying in Docker, Heroku or similar systems but as you say, there are many ways to accomplish it.

On Mon, Jul 3, 2017 at 4:55 AM, Cezary Statkiewicz ***@***.*** > wrote: NOT having "geonode.settings import *" in .setting was by design and not an error, so to avoid circular imports, etc. Didn't we just have a thread about this last week or something? I'm not aware of such discussion. Could you point me to some archives? But anyway, approach introduced in master seems to be wrong for several reasons: - this is code duplication of core element, which is bad in itself. Geonode's settings contain many entries that are geonode-specific, and usually they are good defaults (unless one knows what's doing, no point to change it). The same goes to urls. This stuff is usually too sensitive to change whole file 'just because'. - I believe most common usage scenario for geonode-project is to have customized html templates, and introduce additional functionalities. In this case, there's no point to duplicate good defaults to geonode-project, only things that are really needs to be changed to have deployment working for user without much of fuss. Bigger customizations require more work, geonode-project won't be helpful then anyway. - It is common practice to have base settings and deployment-specific settings that import base and change just few things. Using geonode.settings as a base is also more future-proof than having settings duplicated to geonode_project.settings and using that as a base. Geonode's settings change between versions, user would change geonode's required version, ending up with old, potentially broken settings. No need to make things harder than they should be, really. - Apart from practical inconvenience of specifying env variables to run django app, using environment variables in settings is not very wise, when you operate with complex data structures. With env variable you can pass a string, but not dictionary or a list. I don't think things like https://github.com/GeoNode/geonode-project/blob/ 791f791/project_name/settings.py#L363 <https://github.com/GeoNode/geonode-project/blob/791f791d9edb8b59ef7d4e5c04ba9d2e8d1331b5/project_name/settings.py#L363> would ever work with env var. Also, from devops perspective: Django provides configuration mechanism already with a separate settings file. No need to reinvent the wheel here with os.environ. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#21 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADW1ycKpzOJiUedCLCfuFwlK0LRthH4ks5sKKx2gaJpZM4OKvgx> .

[kalxas]

+1 to have a dev call about this.
@cezio the discussion you are missing is here:
#18

[rob-metalinkage]

Hi
am writing additional modules for a downstream geonode project - to add a layer of semantic enablement, but these are in fact standalone django modules that have already been deployed against geonode installs. I'd like to have a clear instruction to developers of add-on functions about how to present module-specific defaults and customisation options into a django parent project in general and geonode specifically.
Happy to discuss, review proposals and be a guinea pig to test such ideas out.

[cezio]

@rob-metalinkage if this what you're writing is just Django app, this can be plain package (proper setup.py + app code). No need to use geonode-project here, especially if you don't customize base templates.

For module-specific defaults and settings, check https://django-appconf.readthedocs.io/en/latest/

[rob-metalinkage]

each package has setup, and can be installed into python environment and manually configured into any django project. AFAICT this doesnt however address the issue of settings required for a module, installing other components a module might need to communicate with, or running tests against modules. hacking the geonode (or other master project) settings and urls seems to be the only option.
(IMHO the way migrations are picked up for each module should be extended to settings templates and tests - maybe I'm missing something but havent been able to find anything about this)

[afabiani]

@ALL latest agreements have been reflected on this PR

#24

please find also Hangout discussion minutes here

https://goo.gl/cGHznN

[afabiani]

@simod you merged the wrong one