Uploaded image for project: 'SX Spring Python'
  1. SX Spring Python
  2. SESPRINGPYTHONPY-128

Cleanup CherryPy-specific classes, dependencies and overall hierarchy

    Details

    • Type: Refactoring
    • Status: Closed
    • Priority: Major
    • Resolution: Complete
    • Affects Version/s: None
    • Fix Version/s: 1.1.0.M2
    • Component/s: Core
    • Labels:
      None

      Description

      >Hi,
      >
      >during the cleanup and packaging of springpython and seminode I raised several
      >exceptions targeting cherrypy and S3 code in the springpython.security module.
      >
      >First question in general: What should be the policy?
      >
      >Should there be strong dependencies to other frameworks or should SP running
      >with the smallest possible dependency list?
      >
      >>From the point seminode (our upcoming framework) I'm interested in the
      >smallest possible list. Also for packaging reasons it would be much more
      >helpful or at least to split high dependency code from generic ones.
      >
      >What's your mind?
      >
      >I will announce a Debian/Ubuntu Repository the next days where I provide debs
      >for springpython and seminode.
      >
      >The Repository will be apt-get (able) and will contain stable and developer
      >versions.
      >
      >Regards
      >
      >Sven

      1) I searched the code base, and didn't find any dependencies in the src/springpython regarding S3. The only usage of S3 I see is inside build.py (as it should be). build.py is built to upload built packages to S3. I know that coily also has dependencies on S3, but not using the coded API, but instead the S3 browsing site. This is so that coily can download plugins from S3 (though other locations can be added in the future). I can see that coily has a commented-out S3 import statement. This should be removed.

      2) Regarding CherryPy dependencies, overall philosophy is that it should be an option for the user to use CherryPy, not a requirement. I want this to be the case for any external library we link to. Spring Java has worked hard to minimize external dependencies, and I would like to do the same. I know that the security.web components I have built so far have only been tested against CherryPy. I have tried to create things like RedirectStrategy and SessionStrategy interfaces, and then have a concrete instance for CherryPy's web framework. Ideally, these concrete classes should be the ones actually importing cherrypy. That way, if you never use that instance, you won't get hit with a dependency issue. I'm know that this is NOT the case right now. For a good example of how this type of issue was done the right way, look at springpython.database.factory. That has multiple connection factories in one file, with the import statements inside the classes. Don't use the class; don't do the import.

      Due to security.web's need for multiple support classes, it seems best to:
      a) move any/all CherryPy concrete classes from security.web into a security.web.cherrypy31 (which I know is not used right now). I think the right ones look like CP3xyz.
      b) renamed security.web.cherrypy31 to security.web.cherrypy. Until we need to split multiple versions into multiple files, we shouldn't constrict ourselves like that.

      Since I'm sure your questions tie in with debian packaging, I would suggest making things like CherryPy a suggestion and not a requirement, and let's spend the rest of the effort fixing up the source code to support that.

      build.py and S3.py shouldn't be part of the debian distribution. Source code and coily should.

        Activity

        Hide
        gregturn Greg Turnquist added a comment -

        Successfully passed all tests.

        Show
        gregturn Greg Turnquist added a comment - Successfully passed all tests.

          People

          • Assignee:
            gregturn Greg Turnquist
            Reporter:
            gregturn Greg Turnquist
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: