cms-dev / cms

Contest Management System
http://cms-dev.github.io/
GNU Affero General Public License v3.0
904 stars 361 forks source link

Split out the "executable" part of the source #85

Open lw opened 11 years ago

lw commented 11 years ago

Right now in the module for each service there's a global 'main' function that gets executed when the module is launched from the command line as a Python script. We then instruct setuptools to automatically create and install console scripts as entry points for these functions. I don't think it's the correct approach.

I suggest to move out all these 'main' functions from their modules and put them each in a separate file inside a new $REPO/bin directory and tell setuptools to just use these scripts as "entry points" (thus installing them in /usr/bin) instead of creating its own ones. This is possible using the 'scripts' keyword of the 'setup' function. (This keyword is already defined by distutils, and is not an extension introduced by setuptools like 'entry_points' is) (The distutils documentation suggests the $REPO/scripts directory, yet I prefer 'bin'...)

This approach has no clear practical advantage: I propose it just to separate the code that "does things" from the code needed to initialize it (e.g. parsing command line arguments, etc.). One small practical advantage would be a simpler and more robust solution for issue #79, since all executable files would be in the same directory.

This change doesn't only apply to services. Other tools, like the ones in cmscontrib, could be moved to 'bin'. We could for example have YamlImporter and Reimporter become simple executable scripts which call, in slightly different ways, functions and classes in a new module (called for example YamlLoader) placed in cmscontrib, which does all the dirty work. (This is related to the change I proposed in #71)

What do you think?

giomasce commented 11 years ago

No objections from me.

lw commented 11 years ago

Together with the fix to this issue we could also remove the shebang from all modules not intended to be executed (i.e. everything except those that will be put inside bin or scripts - we still have to decide the name).

giomasce commented 11 years ago

I would leave the shebang, because it helps identifying that file as Python source code.

giomasce commented 11 years ago

Apparently gevent's monkey patching has to be done in the main module of a program. If not, the program spits out strange errors just before terminating (thay seem to be harmless, but are annoying anyway). So, implementing this proposal and executing directly CMS programs instead of passing through distutils stubs would fix this problem. So +1 for solving this issue!

lw commented 11 years ago

Started in 608ef7ea983d61e6614c02d1e07a6c57bea2c900. Didn't work as expected (the files in /usr/bin are still generated by setuptools) but it seems that the issue reported by Giovanni in the previous post is fixed. Keeping this open as other commands still need to be ported.