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

Revive Django Plugin! #25

Closed
aelgru opened this issue Nov 21, 2013 · 14 comments
Closed

Revive Django Plugin! #25

aelgru opened this issue Nov 21, 2013 · 14 comments

Comments

@aelgru
Copy link
Contributor

aelgru commented Nov 21, 2013

What are we going to do with the python.django plugin?
It's not doing a lot right now ...and what it is doing is buggy.

@mriehl
Copy link
Member

mriehl commented Nov 26, 2013

I'm not a huge django fan (or user) so I don't really care.
What is it that requires a special plugin treatment? Is manage.py not sufficient?

@mriehl mriehl mentioned this issue Dec 9, 2013
15 tasks
@mriehl
Copy link
Member

mriehl commented Dec 9, 2013

One other thing:

I just realized this plugin provides a task that blocks (until you kill the server, that is). This is contrary to the assumption that tasks are small building blocks that can be combined in a specific order to form a build process.
I think a blocking task is a no-no and we should just remove the plugin.

@zenweasel
Copy link

The only reason I looked at this project (and tested it) was because of the django plugin listed. Even though you are not a "fan", the Django community is a large part of the Python development community and not making it easy to snap in projects will mean that this is just another build tool that no one uses. Which is too bad because its very well designed.

I think the main thing that a plug for Django would provide would be some reduction of the very deeply nested file structure and make it easier to maintain a standard Django project layout..

Yes I am willing to do the work here, but not if the primary maintainers couldn't give a hoot about Django.

@aelgru
Copy link
Contributor Author

aelgru commented Dec 29, 2013

Hi @zenweasel,

thank you very much for your feedback! I am very happy to hear some ideas about what the plugin could do.

I have been using Django a few times and actually I think it's a very nice framework. Once I tried to introduce PyBuilder to a existing Django project. We really wanted to use PyBuilder, but unfortunately one of the conventions in Django projects is to put the test code next to the production code into a package called "tests". PyBuilder separates production code, unit tests and integration (web) tests from each other - and I think this is a good idea. But on the other hand side you are absolutly right: a Django developer would expect to be able to simply create a build.py and use the plugin to stick to the Django conventions.

It would be great for us to get a even more detailed view of your idea of the python.django plugin. So, tell us more ...

I think the problem for me and @mriehl is that we are both not working on Django projects right now ... but we would really be happy to turn the plugin into something great. What do you think about working on this together?
We could create an "experimental fork" within the organization.

Thanks again for your feedback!

@zenweasel
Copy link

Hi @aelgru, thanks for an enthusiastic and positive response to my query.

The current version of Django no longer requires that you place your tests anywhere is particular except inside the project. Most people do structure their project so that tests for an app are in that app but there is nothing but good organization that requires it. Django just uses the native unittest Discover functionality to do test discovery.

Yes I would be glad to try and do what I can do to create a working Django plugin. I need to do some more intense reading of the code to understand all this going on and what side-effects of changing the default paths are.

Again, this looks like a great project. I tried out the livestatus example and the one-liner from empty directory to running project was very impressive.

@mriehl
Copy link
Member

mriehl commented Dec 29, 2013

Hi @zenweasel ,

I would like to apologize in case my statement sounded exclusive or negative to you.
It's just that I barely know django and the current best practices.

If you are willing to help us out figuring out what the plugin needs to do and how, I'm totally up for working on such a plugin!

@aelgru
Copy link
Contributor Author

aelgru commented Dec 29, 2013

While thinking about the topic I started to remember why it was such a pain the last time I tried to adapt the plugin to the Django conventions.

Right now PyBuilder is discovering (source, test, scripts ...) files using directories. See expand_path in core.py
I believe a better choice would have been globs. This would give us the required flexibility. From my point of view this means we will have to refactor the Project class in core.py and every usage of expand_path. An interesting question is going to be how to keep backwards compatibility... any other ideas / suggestions?

@zenweasel
Copy link

Just wanted to check in and saying that I am still working on this, but slowly. Has not been abandoned.

@aelgru aelgru changed the title What are we going to do with the python.django plugin? Revive Django PlugIn! Apr 8, 2014
@aelgru
Copy link
Contributor Author

aelgru commented Jun 29, 2014

I've started to work on an external plugin at https://github.com/pybuilder/pybuilder_django_plugin

@aelgru
Copy link
Contributor Author

aelgru commented Aug 27, 2014

I was thinking about this for a while now. Somehow I believe the only way we can make sure that PyBuilder will be able to distinguish source and test files in a Django project will be by patching the corresponding functions in the plugin ... or regex? ... or maybe we need configurable / extendable file filters, like ...

def django_source_file_filter(file_path):
    if is_djang_source_file(file_path):
         return True
    return False

@init
def init(project):
    project.set_property('source_files_filter', django_source_file_filter)
    project.set_property('test_files_filter', ...)
    project.set_property('script_files_filter', ...)

Which should then be used by PyBuilder instead of default filters (which would be the glob filters).

At least globs are not sufficient since you somehow have to make sure that everything that looks like a test file / module are not treated like source files.

@mriehl
Copy link
Member

mriehl commented Aug 29, 2014

I'm not sure I understand how a test file might be treated as a source file. Is this a problem with the way django itself finds files? Why is the glob not enough?

@aelgru
Copy link
Contributor Author

aelgru commented Aug 29, 2014

Please excuse the short answer, but I am at lunch right now. 🍝 🍵

Unfortunately it's a Django convention to place "test files" into the "production code" package:

aelgru $ django-admin.py startapp example
aelgru $ tree example/
example/
├── admin.py
├── __init__.py
├── models.py
├── tests.py
└── views.py

Another example:
backbone_example.tweets

One purpose of the Django plugin has to been to be able to stick to the Django conventions.
As far as I know it's not possible to exclude files using globs. In general I believe it would be best if a plugin could provide it's own "source / test / script file discovery strategy".

@mriehl
Copy link
Member

mriehl commented Aug 29, 2014

Well it was mentioned that django itself uses the standard python unittest discovery which is filename-based, so it seemed logical to me that you could map this to a glob.

Is the problem that PyBuilder would package these tests as sources? Or is the problem discovering which files need to run?

@aelgru aelgru changed the title Revive Django PlugIn! Revive Django Plugin! Aug 29, 2014
@aelgru
Copy link
Contributor Author

aelgru commented Aug 29, 2014

We are discussing this since 2013, but it's nice to see you catching up, @mriehl.

But actually it's a nice idea to talk in detail about why we need a plugin for Django.

Setup

Let's checkout the django-backbone-example, create a virtualenv and install pybuilder

git clone https://github.com/joshbohde/django-backbone-example.git
cd django-backbone-example/
virtualenv venv
source venv/bin/activate
pip install pybuilder

Fail Fast

Of course executing pyb will fail:

PyBuilder version 0.10.33
Build started at 2014-08-29 22:59:29
------------------------------------------------------------
------------------------------------------------------------
BUILD FAILED - Project directory does not contain descriptor file: /django-backbone-example/build.py
------------------------------------------------------------
Build finished at 2014-08-29 22:59:29
Build took 0 seconds (0 ms)

This error message makes me think, we could give new PyBuilder users a hint that --start-project exists.

--start-project

But since I am a Django developer pyb --start-project now confuses me:

Project name (default: 'django-backbone-example') :            
Source directory (default: 'src/main/python') : backbone_example
Unittest directory (default: 'src/unittest/python') : backbone_example
Scripts directory (default: 'src/main/scripts') : backbone_example
Use plugin python.flake8 (Y/n)? (default: 'y') :  
Use plugin python.coverage (Y/n)? (default: 'y') : 
Use plugin python.distutils (Y/n)? (default: 'y') : n

Django projects don't have scripts. How do I tell --start-project to ignore that? The default makes it impossible to not set any directory for scripts.

But --start-project works and gives me something to start with, build.py:

from pybuilder.core import use_plugin, init

use_plugin("python.core")
use_plugin("python.unittest")
use_plugin("python.install_dependencies")
use_plugin("python.flake8")
use_plugin("python.coverage")


name = "django-backbone-example"
default_task = "publish"


@init
def set_properties(project):
    project.set_property("dir_source_main_python", "backbone_example")
    project.set_property("dir_source_unittest_python", "backbone_example")
    project.set_property("dir_source_main_scripts", "backbone_example")

Try again

Let's remove the last line from build.py, run pyb install_dependencies and try to execute pyb again.

PyBuilder version 0.10.33
Build started at 2014-08-29 23:06:34
------------------------------------------------------------
[INFO]  Building django-backbone-example version 1.0-SNAPSHOT
[INFO]  Executing build in aelgru/django-backbone-example
[INFO]  Going to execute task publish
[INFO]  Executing unittest Python modules in aelgru/django-backbone-example/backbone_example
[WARN]  No unittests executed.
[INFO]  All unittests passed.
[INFO]  Building distribution in aelgru/django-backbone-example/target/dist/django-backbone-example-1.0-SNAPSHOT
[INFO]  Copying scripts to aelgru/django-backbone-example/target/dist/django-backbone-example-1.0-SNAPSHOT
[INFO]  Collecting coverage information
[INFO]  Executing unittest Python modules in aelgru/django-backbone-example/backbone_example
[WARN]  No unittests executed.
[INFO]  All unittests passed.
[WARN]  Module not imported: manage. No coverage information available.
[WARN]  Module not imported: urls. No coverage information available.
[WARN]  Module not imported: __init__. No coverage information available.
[WARN]  Module not imported: settings. No coverage information available.
[WARN]  Module not imported: local_settings. No coverage information available.
[WARN]  Module not imported: tweets.views. No coverage information available.
[WARN]  Module not imported: tweets.urls. No coverage information available.
[WARN]  Module not imported: tweets.models. No coverage information available.
[WARN]  Module not imported: tweets. No coverage information available.
[WARN]  Module not imported: tweets.tests. No coverage information available.
[WARN]  Module not imported: tweets.tests.gravatar. No coverage information available.
[WARN]  Module not imported: tweets.templatetags.verbatim. No coverage information available.
[WARN]  Module not imported: tweets.templatetags.straight_include. No coverage information available.
[WARN]  Module not imported: tweets.templatetags. No coverage information available.
[WARN]  Module not imported: tweets.templatetags.mustache. No coverage information available.
[WARN]  Module not imported: tweets.api. No coverage information available.
[WARN]  Module not imported: tweets.api.api. No coverage information available.
[WARN]  Module not imported: tweets.api.resources. No coverage information available.
[WARN]  Overall coverage is below 70%:  0%
------------------------------------------------------------
BUILD FAILED - Test coverage for at least one module is below 70%
------------------------------------------------------------
Build finished at 2014-08-29 23:06:35
Build took 1 seconds (1112 ms)

Of course PyBuilder could not detect any tests.

What now?

How can we make sure PyBuilder understands what are source files and what are test files?

Test Files

To find **/tests/*.py looks promising, but it isn't since it's not the file structure, but the package that counts.
It's more something like **/tests.py,**/tests/*.py or at least it's multiple globs ... and of course PyBuilder has to respect that not every file that matches **/tests/*.py is a test file. But well, this is a (somehow) manageable problem.

Source Files

Nevertheless we have another problem: if you look at the output you can clearly see that PyBuilder thinks that tweets.tests is a source directory.
One of PyBuilders file structure concepts is that source and test sources are strictly separated - and I like that. You can execute flake8 on sources without having to run it on your test files. 👍

So how will you exclude files using glob? As far as I know this is not possible.

@mriehl mriehl removed this from the v0.11.0 milestone Oct 21, 2015
@aelgru aelgru closed this as completed Jun 11, 2016
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

4 participants