[WIP] New project layout

[AlexHill]

First attempt at this here. Haven't gone over settings or fabfile in detail yet, so I'm sure some things won't work. I also want to update settings to their current defaults in Django, where relevant.

In order to put the project_template in a Django-compatible format, I've renamed local_settings.py.template to just local_settings.py in the project app directory (deploy/local_settings.py.template remains).

This means that .template files are exclusively for the use of the fabfile, which I think is nice. But it changes how tests work, so you'll have to have a look over that and see what you think.

[stephenmcd]

Awesome Alex.

What do you mean by "Django-compatible format", or more specifically, is there anything preventing the local_settings file from continuing to use the ".template" suffix, other than the fact we'd need to extend the command a bit further to rename the file once it's been copied over?

Personally (in the literal sense) I'd like to keep the name as is - it's very convenient to be able to treat the project template dir as a local site during development. I keep a "local_settings.py" file in there that never gets checked into source control. Otherwise I and anyone else working on Mezzanine will need an actual separate project to test with - I assume a lot of people contributing do that already, but I've never had to.

[AlexHill]

By "Django-compatible", I mean a project template that can be fed directly to Django's startproject (with the right parameters) and generate a correct project without having to run any extra code.

But now that I think about it, I think we can actually pass parameters that make startproject rename local_settings.py.template for us, because it already has the facility to rename files and folders, as in the project_name directory. I'll check it out.

[stephenmcd]

Good find :-)

[AlexHill]

Hmm nope, I read it wrong. The only thing that gets replaced in paths is the string "project_name" with the project's actual name.

We can replace the file in our management command, but we'll have to duplicate the code that Django uses to ascertain the target directory. That code (and the names of various arguments) differs between 1.7 and 1.8, which is rather annoying. I'll get back to you :)

[stephenmcd]

Maybe there's another way we can treat the project template as a local site and somehow have it use uncommitted local settings, possibly outside of local_settings.py

I'll have a think too.

[AlexHill]

OK I think the latest commit should do it. Creates a correct project structure in 1.7 and 1.8.

In the deploy directory, is there any reason for the inconsistency in the ".template" naming? All the files are templates, but only the .py files are named as such. Can we either add it to all or drop it entirely?

[stephenmcd]

Cool!

I vaguely recall it would prevent the pep8/pyflakes tests from trying to validate those files, which would fail since they contain placeholder strings in them and therefore aren't valid Python files.

But making them all .template files seems fine.

[AlexHill]

Ah, that makes sense.

Speaking of pyflakes - what do you think about running flake8 and isort over the codebase with the same configuration as Django? The biggest implication there is increasing the max line length to 119. Overall I think we should aim to reduce friction as much as possible for people familiar with Django.

Edit: I mean run them as part of test runs on Travis.

[stephenmcd]

Definitely - the code currently supporting pep8/pyflakes is really bad and I was looking forward to throwing it out in favour of flake8, you can see I started some of the configuration in https://github.com/stephenmcd/mezzanine/blob/master/setup.cfg

Just never got around to finishing it.

[AlexHill]

Excellent :)

On Mon, Apr 13, 2015 at 1:27 PM, Stephen McDonald [email protected]
wrote:

Definitely - the code currently supporting pep8/pyflakes is really bad and
I was looking forward to throwing it out in favour of flake8, you can see I
started some of the configuration in
https://github.com/stephenmcd/mezzanine/blob/master/setup.cfg

Just never got around to finishing it.

—
Reply to this email directly or view it on GitHub
#1265 (comment).

[auvipy]

will be merged in new release?

[stephenmcd]

Possibly, can't say for certain - if not, certainly the next release after that.

[AlexHill]

Ok, fabric is working, I think. Needs a bit more testing but I wanted to get these changes up.

[AlexHill]

Phew, OK. I think we just need an audit of the default settings.py and this will be good to go.

[Kniyl]

Seems nice. You may want to change settings.SITE_ID and settings.BASE_DIR according to https://github.com/django/django/blob/1.7.1/django/core/checks/compatibility/django_1_6_0.py in order to be closer to pure django projects.

[AlexHill]

Yes, we should go through the settings file with the goal of making it minimally different from Django's default settings.

Fabric support in this branch is solid now.

[stephenmcd]

Can we squash these into a single commit?

[stephenmcd]

I've tested this against fabric with a new VM and all works fine. Had a couple minor issues - I can commit fixes once this is merged, or Alex you could include it in the squashed commit, neither here nor there.

Fixes a unicode error I get with mezzanine-project:

--- a/mezzanine/bin/management/commands/mezzanine_project.py    Sat Jun 06 12:48:12 2015 +0800
+++ b/mezzanine/bin/management/commands/mezzanine_project.py    Wed Jun 10 15:27:33 2015 +1000
@@ -64,5 +64,11 @@
         """Use Mezzanine's project template by default. The method of picking
         the default directory is copied from Django's TemplateCommand."""
         if template is None:
-            return path.join(mezzanine.__path__[0], subdir)
+            full_path = path.join(mezzanine.__path__[0], subdir)
+            try:
+                # Python 2
+                return unicode(full_path)
+            except NameError:
+                # Python 3
+                return full_path
         return super(Command, self).handle_template(template, subdir)

Error is the same as https://code.djangoproject.com/ticket/18091 - I guess we regressed it somehow by overriding that method, but the above fixes it.

I also found it difficult to develop Mezzanine from within itself (eg using "project_template/project_name" dir as the actual project), with {{ project_name }} used throughout various files. If that was just limited to local use of manage.py we could probably get away with passing --settings=project_name.settings, but it also prevents testing the deploy process without having a generated project.

So I think we could live without fixing that if there wasn't an easy fix, but I think it would be easy enough to just create a function (say in mezzanine.utils.conf) that sets the environment variable, and is called from manage.py, wsgi.py, etc, where we'll still use {{ project_name }}, but in the case of developing locally, we actually check to see if the value is literally {{ project_name }} and if so, just use project_name instead (being the literal name of the dir). Some code might explain it clearer:

def set_settings_module(project_name):
    if project_name == "{{ project_name }}":
        # project files haven't been templatised so we're developing 
        # internally, just use the literal "project_name" directory
        project_name = "project_name"
    os.environ.setdefault("DJANGO_SETTINGS_MODULE", "%s.settings" % project_name)

Then in manage.py etc, we just do set_settings_module("{{ project_name }}")

[AlexHill]

Ah yeah. I ran into that bug as well, but I fixed it in my local Django and proceeded to forget about it. I think it's a legitimate separate bug in Django, triggered by unicode file names rather than content as in that Trac ticket. Your fix looks fine to me. When I have some more time I'll submit a patch to Django as well.

Second problem: approach sounds fine, but I think the utility function should just return a string, rather than actually setting the env var, and then we simply use that wherever we would have used "{{ project_name }}". I think that flexibility is necessary here, for example.

[stephenmcd]

but I think the utility function should just return a string, rather than actually setting the env var

Nice - let's do that

[AlexHill]

OK I've added that function and used it everywhere I think it's necessary, and committed that unicode fix. Sorry for the noise, I'm editing from the web.

@stephenmcd if you can test those things again I'll squash when I'm back on my dev machine.

[stephenmcd]

Looks good - unicode fix is better than my one :-)

[AlexHill]

I've added BASE_DIR and I think that will do for now. We need SITE_ID since we use django.contrib.sites unconditionally.

That should be enough to merge, but I think the settings file could still use a bit more attention.

[AlexHill]

Glad this is merged. I was gonna squash this and #1326 before merging – too late now they're in master?

[stephenmcd]

Yeah I totally forgot about squashing, but this really forces us to start
using a manual changelog, which is good.
On 11/06/2015 12:15 PM, "Alex Hill" [email protected] wrote:

Glad this is merged. I was gonna squash this and #1326
#1326 before merging – too
late now they're in master?

—
Reply to this email directly or view it on GitHub
#1265 (comment)
.

[molokov]

Alex, what's the replacement for the -a option in mezzanine-project? (e.g. before we would do mezzanine-project -a cartridge, how is that done now? Does cartridge still need to be updated to the new format and its doc changed?)

[AlexHill]

Does cartridge still need to be updated to the new format and its doc changed?

Yep.