On 07/13/2010 12:57 PM, Sebastian Benthall wrote:
I think that makes a lot of sense.

Could explain what's going on with gsconfig.py and its inclusion in the build? What's the commit policy on that like?
gsconfig is currently included in the build through pip's SCM integration (pip does a checkout of gsconfig master and builds from that). This is basically a dependency on an un-pegged revision (kind of bad for reproducable builds, but analogous to what we're doing with maven snapshots.) It might be smart to switch over to a git submodule for this.
It sounds like there will be lots of times when the right thing to do for GeoNode development will be work on the gsconfig.py, but the relationship between the two projects is at this point somewhat ambiguous to me.
Luke, and Ivan have free commit rights to the gsconfig repository on github. We could fork it under the GeoNode account if we want; I don't see any good arguments for or against that approach. I'm happy to fix any bugs reported and review any patches provided.

Could you provide an example of work that didn't happen, or happened in a 'wrong' way, because it required changes to gsconfig?

--
David Winslow
OpenGeo - http://opengeo.org/

    I think if we exposed two properties (just two) that represent
    "geonetwork stuff" and "geoserver stuff" respectively on layers
    then it would serve as a reasonable flag that "hey this isn't just
    another field from the database record".

    So instead of:

    >>> layer.styles # secretly there's an HTTP request and an XML
    parse in here, that stuff takes time

    it's

    >>> layer.gsconfig.styles # now it's explicit, or at least
    something a reviewer can grep for

    This also has the side effect of delegating the GeoServer stuff to
    the gsconfig API, and the GeoNetwork stuff to the
    geonode.geonetwork API, so as new fields are added or whatever we
    don't have to maintain a bunch of boilerplate in the models.

    I don't think the current approach of "save everything to
    geoserver/geonetwork if those records are dirty when we save to
    the database" is particularly problematic, but I guess time will
    tell on that one.

    --
    David Winslow
    OpenGeo - http://opengeo.org/


    On 07/13/2010 12:15 PM, Sebastian Benthall wrote:
    Thank you very much for this explanation.  I understand the
    problem much better now.

    I think it would be a good idea to settle the point re: Note 3
    and its relationship to all GeoServer and GeoNetwork properties
    as soon as possible, because, as you say, it's harder to change
    API's that are already in use.

    What do you think the right way to go about resolving that
    question is?

    On Tue, Jul 13, 2010 at 10:38 AM, David Winslow
    <[email protected] <mailto:[email protected]>> wrote:

        On 07/12/2010 06:29 PM, Sebastian Benthall wrote:

            These shortcut methods will error if, for whatever
            reason a layer, has
            more than one poc or metadata_author.  It doesn't seem
            unreasonable to
            expect multiple of either, so if we are going this way
            then we should
            probably ditch the properties and make developers deal
            with the idea
            that there might be more than one party playing any
            particular role for
            any particular layer.  Otherwise, we should add a
            uniqueness constraint
            on ContactRole on (layer, role).


        We had a similar discussion, if I recall correctly, about
        the use of shortcut property methods on the Layer model to
        access information from the Catalog.  If this is something
        that is going to keep coming up, it would be good to talk
        about it and arrive at a consensus.

        So, I'm going to ramble on it for a while.


        Personally, I'm in favor of shortcuts as long as they don't
        error.  I don't think "making the developer deal with the
        idea" in software design is a much more productive attitude
        than "make the user deal with the idea" in user interface
        design.

        I certainly can agree to disagree on this.  And David, if
        you think its very important and especially if you see a
        technical reason for it, I wouldn't be opposed to some kind
        of coding policy around it.  But from my vantage point, it
        seems on one level like an unsubstantive issue of coding style.

        But I can think of one practical reason to encourage these
        kinds of shortcuts.  it seems like the
        heavy-model/light-view coding pattern and the use of
        @property decorations as a shortcut for field access are
        common expectations for Python/Django developers (somebody
        who knows better what they are talking about should feel
        free to correct me).  I wouldn't want our code to alienate
        potential contributors who are attracted to the project
        because they are good at customizing Django apps.

        So I am in favor of not restricting this sort of thing, at
        least until we can see tangibly where it becomes a problem
        (we can always change it then).

        If, on the other hand, we decide to take a position against
        property decorations on the Models, it would be good to have
        a policy about it that can be communicated in our coding
        standards rather than just strongly encouraged during code
        reviews.  We should then refactor the existing code to be in
        line with those standards.  Otherwise, I think this is going
        to be a source of confusion and possibly frustration for
        contributors moving forward.

-- Sebastian Benthall
        OpenGeo - http://opengeo.org

        A few points:

        1) The property in question is not just unnecessary, it is
        incorrect.  The incorrectness is a much bigger deal (to me)
        than the property-ness.

        2) Good API design, like good UI design, is all about
        choosing the proper level of abstraction (and good
        naming/labelling, and all of that. but the abstraction is
        key.)  The proper level of abstract is not always the one
        that hides the most information.  It is ridiculous to write a
        model method for every possible query on related objects
        (foreign keys/many-to-many relations) we might want to do,
        when we have a nice simple ORM method for querying by
        arbitrary criteria.

        2b) It is much tougher to remove methods from an API that is
        in use than to add them.

        3) The official Python style guide
        (http://www.python.org/dev/peps/pep-0008/) provides these
        guidelines regarding the use of properties:

            Note 1: Properties only work on new-style classes.

            Note 2: Try to keep the functional behavior side-effect
            free, although
            side-effects such as caching are generally fine.

            Note 3: Avoid using properties for computationally expensive
            operations; the attribute notation makes the caller believe
            that access is (relatively) cheap.

        I don't think that we need to address properties beyond this
        in our own style guidelines.  However, Note 3 (debatably)
        applies to all of the GeoServer and GeoNetwork access properties.

        4) One technical argument against using annotations in
        general is that annotation syntax a new Python feature in
        version 2.6.  @property does not work in some alternative
        Python implementations, notably Jython.  This is not an
        argument against properties themselves since decorators are
        pure syntactic sugar.




-- Sebastian Benthall
    OpenGeo - http://opengeo.org





--
Sebastian Benthall
OpenGeo - http://opengeo.org


Reply via email to