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

Split geoserver.helpers.set_attributes() #2699

Merged
merged 8 commits into from
Nov 8, 2016

Conversation

JivanAmara
Copy link
Contributor

Split this function into two with one containing geoserver-agnostic logic, and the other home to the geoserver-specific logic. This is support for django-osgeo-importer changes which will allow imports that don't require a geoserver instance.

signals.post_delete.connect(post_delete_site, sender=Site)
signals.post_save.connect(post_save_profile, sender=Profile)
signals.post_delete.connect(post_delete_profile, sender=Profile)
if 'geonode.contrib.geosites' in settings.INSTALLED_APPS:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be if 'geonode.geoserver' in settings.INSTALLED_APPS I believe

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is due to tests failing with the included settings file because the models for geosites aren't created in the database (the default settings doesn't include geosites) but the signals still get registered.

@@ -695,3 +698,86 @@ def check_shp_columnnames(layer):
qry = u"ALTER TABLE {0} RENAME COLUMN \"{1}\" TO \"{2}\"".format(inLayer.GetName(), key, list_col[key])
inDataSource.ExecuteSQL(qry.encode(layer.charset))
return True, None, list_col


Attribute = None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be global?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out not needed & removed in latest commits.

@jj0hns0n
Copy link
Member

jj0hns0n commented Nov 5, 2016

@ingenieroariel can you help @JivanAmara with this PR. I've raised a few questions.

@simod
Copy link
Member

simod commented Nov 7, 2016

Hm weird. We should investigate more why this is happening. Those signals
should be used only if geosites is in installed apps.

Il lunedì 7 novembre 2016, Jivan Amara [email protected] ha
scritto:

@JivanAmara commented on this pull request.

In geonode/contrib/geosites/models.py
#2699:

@@ -98,13 +99,14 @@ def post_delete_profile(instance, sender, **kwargs):

Django doesn't propagate the signals to the parents so we need to add the listeners on the children

-signals.post_save.connect(post_save_resource, sender=Layer)
-signals.post_save.connect(post_save_resource, sender=Map)
-signals.post_save.connect(post_save_resource, sender=Document)
-signals.post_save.connect(post_save_site, sender=Site)
-signals.post_delete.connect(post_delete_resource, sender=Layer)
-signals.post_delete.connect(post_delete_resource, sender=Map)
-signals.post_delete.connect(post_delete_resource, sender=Document)
-signals.post_delete.connect(post_delete_site, sender=Site)
-signals.post_save.connect(post_save_profile, sender=Profile)
-signals.post_delete.connect(post_delete_profile, sender=Profile)
+if 'geonode.contrib.geosites' in settings.INSTALLED_APPS:

No, this is due to tests failing with the included settings file because
the models for geosites aren't created in the database (the default
settings doesn't include geosites) but the signals still get registered.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#2699, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAvkdL6ZVSVPp421a5_qhl3gSMJfkexpks5q74k7gaJpZM4KqKnU
.

Simone

@JivanAmara
Copy link
Contributor Author

I think with signals, geosites won't send any signals if it's not in INSTALLED_APPS, and won't create database tables for its models, but if geosites.models is imported anywhere, the handlers listening to signals from other apps will still get registered.

@simod
Copy link
Member

simod commented Nov 8, 2016

I see and what is the case where geosites are imported but not installed?

@JivanAmara
Copy link
Contributor Author

Sorry, I didn't make a note of the stack traces that led me to make that change. It's easy to reproduce though; check out the version before that commit & make your local_settings import everything from settings then run tests.

@simod
Copy link
Member

simod commented Nov 8, 2016

Sorry I don't get it. The tests are passing on master but what's the reason why local_settings should import from settings? This shouldn't happen.

@ingenieroariel
Copy link
Member

@simod, I don't see local_settings importing settings in the PR but I can easily see how geosites could have registered signals even if it wasn't imported (by some other app importing from geosites).

I plan to merge this one today unless there are objections. From me, the only part I find weird is having to import comments or ratings on the geoserver helper but we can deal with that in the future.

@ingenieroariel ingenieroariel merged commit 5dd5cec into GeoNode:master Nov 8, 2016
@JivanAmara
Copy link
Contributor Author

JivanAmara commented Nov 8, 2016

@simod, do you understand that I'm talking about running the tests locally? After cloning into a fresh VM, setting up dependencies, a database, and a geonode instance running tests (python manage.py test) results in mostly errors:
EEEEEEEEEEE...EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE.........EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE....EEEEEEEEE
Adding that conditional makes most of the tests happy again:
..............EEEEEEEEEEEEEEEEE.............E.......................................................................E..............

Perhaps the travis environment is using a different settings file and the tests were assuming the geosites app was installed?

I'd love to dig in deeper and get rid of the rest of those errors so it's easy for a new developer to be able to run tests locally from the get-go, but need to give my attention to some other things.

@JivanAmara
Copy link
Contributor Author

@ingenieroariel, the "import comments or ratings" issue you mention; is this something introduced with my changes? If so I need a little more detail to understand what you're talking about.

@simod
Copy link
Member

simod commented Nov 8, 2016

Sure I understand and I commonly run the tests locally too without errors.
That's why I'm trying to understand what's going on

Il martedì 8 novembre 2016, Jivan Amara [email protected] ha
scritto:

@simod https://github.com/simod, do you understand that I'm talking
about running the tests locally? After cloning into a fresh VM, setting up
dependencies, a database, and a geonode instance running tests (python
manage.py test) results in mostly errors:
EEEEEEEEEEE...EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE.........
EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE....EEEEEEEEE
Adding that conditional makes most of the tests happy again:
..............EEEEEEEEEEEEEEEEE.............E.............................
..........................................E..............

Perhaps the travis environment is using a different settings file and the
tests were assuming the geosites app was installed?

I'd love to dig in deeper and get rid of the rest of those errors so it's
easy for a new developer to get started, but need to give my attention to
some other things.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2699 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAvkdBZlPbDO7bJlC7RkN7VVYoV5DjL1ks5q8MsUgaJpZM4KqKnU
.

Simone

@simod
Copy link
Member

simod commented Nov 8, 2016

Ah now I got it! You run the tests using django test. And this since django
1.8 is traversing the whole structure for files named tests.py. But is not
what should be used to test Geonode. Run paver test, and will run only the
tests in the configured apps.

Il martedì 8 novembre 2016, Simone Dalmasso [email protected] ha
scritto:

Sure I understand and I commonly run the tests locally too without errors.
That's why I'm trying to understand what's going on

Il martedì 8 novembre 2016, Jivan Amara <[email protected]
javascript:_e(%7B%7D,'cvml','[email protected]');> ha scritto:

@simod https://github.com/simod, do you understand that I'm talking
about running the tests locally? After cloning into a fresh VM, setting up
dependencies, a database, and a geonode instance running tests (python
manage.py test) results in mostly errors:
EEEEEEEEEEE...EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE.........EEEEEE
EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE....EEEEEEEEE
Adding that conditional makes most of the tests happy again:
..............EEEEEEEEEEEEEEEEE.............E...............
........................................................E..............

Perhaps the travis environment is using a different settings file and the
tests were assuming the geosites app was installed?

I'd love to dig in deeper and get rid of the rest of those errors so it's
easy for a new developer to get started, but need to give my attention to
some other things.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2699 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAvkdBZlPbDO7bJlC7RkN7VVYoV5DjL1ks5q8MsUgaJpZM4KqKnU
.

Simone

Simone

@JivanAmara
Copy link
Contributor Author

Got it. If I can make some time I'll add a check to warn others so they don't make the same mistake.

milafrerichs added a commit to milafrerichs/geonode that referenced this pull request Nov 14, 2016
*  - Restoring missing parts of the docs
*  - update ext pakages versions

* Lock in version of gsconfig

Lock in the version until issue GeoNode#2544 is resolved

* gsconfig now utilizes an attribution dictionary and bumped to newest version

*  - update geonode exts version in order to fix pavement sync db

* README: fix syntax error

Heading "Docker Usage" was not rendered correctly.

* ol3-preview

* keep geoext as default

* Missing quotation mark

* Add support for EPSG:4326

* bump user-messages

* don't assume 'geoserver' in style url management

* tiles_url should be a XYZ source

* set_metadata returns four variables, which was causing an error of too many values to unpack

* 'name' no longer a ContentType field in Django 1.8+

* Added handling of postgres backed geogig stores

* fix layer style dropdown values

* Default to public schema if not defined

* added exception handler for when multiple legends are returned

* just update the tiles link url if needed, this will respect any changes to the url made by other apps like contrib.mp

* register models for actstream 0.5+ / Django 1.8+

* re enable publish as layer group

* adjusted text and html metadata links in the templates for documents and layers

* Changes in manual installation docs (GeoNode#2561)

minor changes in docs

* fix layer group endpoint

* fix for port changes: can't concatenate str & int

* check if layerfile_set exists

* Refactor to align with updated PG Geogig plugin: calls the plugin only to create the geogig repo, and calls gsconfig to create the datastore

* fix client/server handling for multi-file upload/import by storing the upload id in the session

* use .get explicitly instead of .all().filter(...)[0]

* use elif with mutually exclusive comparisons

* Invalidate layer in GeoWebCache after style_update PUT/POST

* send content-type:text/xml header

* use str.join() properly

* fix status code check

* update url for cache refresh

* prevent auto redirect when additional information is required.

* bump gsconfig to 1.0.6

*  - Paver setup_geoserver using the new 2.9.x

*  - remove layer_is FK constraints from layer_styles migrations

* Added documentation for configuring an AWS S3 Bucket with GeoNode

*  - test travis script with Java 8 (GeoNode#2584)

- test travis script with Java 8

 - test travis script with Java 8

* Adding instructions to add windows dependencies

* Adding gdal environment variables

* Update README

* Update README

* Update README

* Revert " - remove layer_is FK constraints from layer_styles migrations"

This reverts commit ea2b292.

* fix broken og:image link in meta tag in head

* fix exception during renders of admin forms using autocomplete_light

* change LayerStyles related name to snake_case

* Create new CONTRIBUTING file

Using CLAHub and OSGeo CLA

* restructure get_context_resourcetype for clarity

* add profiles & groups autocomplete config

* Groups have titles, not names

* Docker for development (GeoNode#2593)

* Refactor to align with updated PG Geogig plugin: calls the plugin only to create the geogig repo, and calls gsconfig to create the datastore

* prevent auto redirect when additional information is required.

* bump gsconfig to 1.0.6

*  - Paver setup_geoserver using the new 2.9.x

*  - remove layer_is FK constraints from layer_styles migrations

* Added documentation for configuring an AWS S3 Bucket with GeoNode

* fix broken og:image link in meta tag in head

* Revert " - remove layer_is FK constraints from layer_styles migrations"

This reverts commit ea2b292.

*  - test travis script with Java 8 (GeoNode#2584)

- test travis script with Java 8

 - test travis script with Java 8

* No specific versions in setup.py, only minimum

* Bump version to start 2.5 cycle

* Use published image for development

* Added docker-compose and docker-compose.override

* Pinned requirements

* Pull image in deployment, build image in development

* Makefile for tasks

* Added pull target

* Avoid autocompletelight 3

* Switch to 'latest' geonode/docker

* Requirements from travis

* Downgrade requirements

* Use requirements file

* Don't use project_name

* Use specific docker images

* s/genode/geonode

* Use geoserver latest

* Use latest tag for nginx

* Fix migration problems

* Use run instead of exec

* Fix requirements.txt

* Revert "Fix migration problems"

This reverts commit aa33ce6.

* Fix httplib version

* Fix tests

* Call docker-django from latest

* Use geonode/django in celery too (GeoNode#2594)

* make test works in docker (GeoNode#2595)

* Add context-specific search to /groups page

Templates that were originally built for ResourceBase-type content are
being overridden here for group objects (e.g., _group_search_content.html)

* groups do not have date or popular_count fields

* let custom search_content template work for both profiles & groups

* Custom sort filters depending on data type

* Add context-specific search (filtering by username) within /people

* if searching from /people context, use username instead of title

* url fix in README

* add name to AUTHORS

* geonode-user-accounts 1.0.13 (GeoNode#2603)

* Fix for issue 2599. The issue was that apache was allowing directory browsing. Added the options directive to remove it from the current list of options enforced for that directory.

* Use environment variables in settings.py (GeoNode#2596)

* Missing migrations (GeoNode#2604)

* geonode-user-accounts 1.0.13

* Missing migrations

* pep8 for migrations

* pep8

* Updated readme for docker (GeoNode#2607)

* Using docker images from hub

*  - (issue GeoNode#2070) Layer name cannot be a style name already on Geoserver

*  - (issue GeoNode#2075) Add lxml back into paver win_install_deps

* adding missing trans for GeoNode#1769

* sycing tsf

* use layer instance to revoke layer permissions, fixes GeoNode#2531 (GeoNode#2614)

use layer instance instead of resourcebase to revoke permission, fixes GeoNode#2531

* include data files in downstream installations should fix GeoNode#2452 (GeoNode#2615)

include data files in downstream installations should fix GeoNode#2452

* - Modify some settings and code to make geonode able to use QGIS Server as backend.
- Add some views, templates and resources to make geonode able to use QGIS Server as backend.
- Add sample setting to use QGIS Server backend.

* should fix GeoNode#2464 (GeoNode#2616)

bump haystack versions to support django 1.8

* Fixed migrations, settings.py, and pavement for development

* pavement: moved migrate_apps to yaml

* fix GeoNode#1718

* make debug default to true

* sycing for GeoNode#1769, after GeoNode#2612 merge

* fix upload issue, check if the js property exists first

* hierarchical tags support

* fix indentation

* add freeboard to requirements.txt

* bump taggit

* Don't pin versions in setup.py use >= or <=

Pinning should be done in requirements.txt

Reasoning is explained in https://packaging.python.org/requirements/

* Pin versions in requirements.txt

* Set to 2.5 final version for release

* update keywords integration test

* fix filename

* we dont use submodules anymore

* Updated instruction for configure ssl on geonode: with version 2.4 some path are changed (GeoNode#2571)

and is not necessary the step regarding python and java configuration because ssl is managed by apache mod_wsgi

* 2.5.0 is out, move master branch to 2.5.1-dev

* Delete fig.yml

* Do not flake8 migrations

* python-properties is now usually pre-installed in Ubuntu

And GeoNode works well with Ubuntu 14.04

* Fix typos in installation dev docs

* Fix GeoGig stats/logs URL for Layer info page. (GeoNode#2633)

The repoURL was using the GeoServer DataStore connection_parameters
value to contstruct the GeoGig repository URL, which looks like:

   http://<server>/geoserver/geogig/geoserver://<repo name>

The statistics and logs URL for GeoGig needs to use a repo URL like
this:

   http://<server>/geoserver/geogig/repos/<repo name>

* Do not pin versions on setup.py

* Upgrade django mptt

* Use a pattern for adding package data, fixes missing fixtures on packages

* Set development version for 2.5.3

* Add volume data container to geoserver

* Exclude remote services layers from map download feature.

* Make download links open in new tab (GeoNode#2642)

* Fix race condition during Raster upload/import

  - Removing a duplicate block of code for PENDING raster imports
  - Don't re-send the Import request if it's already been sent

* Add variables for building base_url

* local_settings.py should be called only once

* Provide defaults for optional ogc_server_settings (GeoNode#2647)

* Provide defaults for optional ogc_server_settings 

MAPFISH_PRINT_ENABLED, PRINT_NG_ENABLED, GEONODE_SECURITY_ENABLED, GEOGIG_ENABLED

* fixup

* Move master to 2.5.4

* Minor text fix for profile update

* fix QGIS path log when using qgis server backend (GeoNode#2648)

* Bugfix for updating style through UI

* Clear comment modal after adding comment

* Issue GeoNode#2401: GNIP: GeoNode Backup and Restore (GeoNode#2651)

* GeoNode Backup and Restore Management Commands

 - GeoServer Vectorial/Raster Data Full Backup&Restore

 - Backup & Restore Admin Page

 Bakup&Restore Admin Gui migrations

 - Document headers

 - Backup&Restore PR logging and minor fixes

 - fix migration issue

 - Backup & Restore Documentation

 - Minot update Backup & Restore docs

* Update backup.py

* Respond correctly when user removes the last related file for a layer upload (GeoNode#2660)

* bump guardian and manage the anonymous pk internally

* use names instead of slugs for keywords

* ini file was not an allowed file type for MANIFEST.in. This was causing the settings.ini file to not be included in the package.

* Fix small bug where {{ repoUrl }} was being used as geogig email due to incorrect input type in HTML

* Fixing missing and broken links to actions on the user's activity page

* Updating pycsw to the latest released version 2.0.2

* bump pycsw

* Update conf.py

* Update setup.py

* Update conf.py

* Update conf.py

* Update requirements.txt

* Update conf.py

*  - Fix the docs

* Updating Choose Files button to allow user to reupload the same file again without having to use Clear (GeoNode#2681)

* Use new UserActivity view (GeoNode#2680)

* truncate upload names over length to allow upload (GeoNode#2671)

One approach to the problem described in
GeoNode#2646

This patch uses the Django field definition to get the length, so this can be
controlled from that one place and won't get out of sync.

* remove mutable default values for kwargs (GeoNode#2668)

This subtle misunderstanding of Python tends to cause confusing bugs.

Suppose you have a definition statement like 'def function(keys=[]): ...'
This def statement is evaluated once, at import time, to create a function
object. It is at this time that '[]' is evaluated, and it is that ONE
list object which is retained as the default value for 'keys'.
Python doesn't store the expression and re-evaluate it every time the
default is needed, it evaluates it once at import time and the resulting
value is passed for every call where that arg is not given.

To explain this with code, the effect is something like this:

    default_keys = []
    def function(keys=default_keys):
        ...

When function() is called with a value for keys, no problem will be seen.
Even when it is called with no value for keys, and we default to that list,
you might not see a problem. The problem that tends to happen unpredictably
is that the mutable default value is mutated. Those changes are persistent
and globally visible to any call of the function which uses the default.

Then the default value is non-empty. Or information leaks across calls
that shouldn't be leaking. Or the same dict keeps growing forever.

To avoid this, we can use a sentinel value such as None, and then check
for that value in the body of the function. It is a cheap safeguard against
situations that can be very annoying.

* first of many docs fixes (GeoNode#2684)

* Fix the Resources section on profile detail page to work when no data exists in the category yet

* Incubation documentation fixes (GeoNode#2687)

* Fixed dead link [ci skip]

* Fixed some doc links [ci skip]

* Fixed License header in some po files

* Fixed packaging files copyright

* Added OSGeo copyright and CC-SA in documentation

* Fixed bug breaking Group Activity view

* Updated changelog for version 2.5.4

* Updating release info and some improvements to pavement for Xenial releases

* Fixing travis build

* previous adjustments to the setup.py and MANIFEST.ini were not allowing certain filetypes to be packaged, loading.gif was the most noticeable.

* Updated changelog for version 2.5.5

* Update release version

* Make resourcebase info panel template extendable

* Fixing syntax error: celery_app (GeoNode#2697)

* Sync auto generate avatar sizes with avatar sizes used in templates (GeoNode#2693)

*  2696 Part 1 - GNIP - Better management of View and Download permissions.

 2696 Part 1 - GNIP - Better management of View and Download permissions.

 - Fix flake8 formatting issues

* Updated AUTHORS list

* Update views.py

- Fix minor typo with brackets

* Update AUTHORS

* Enable update layers management command to accept the permissions dictionary. (GeoNode#2691)

* Insert missing sudo to chmod commands

This fixes "[Errno 13] Permission denied: '/home/geonode/geonode/geonode/uploaded/layers/" encountered by user when trying to upload layer files.

* Clean up extra ':'s (from ::: to ::)

* Language cleanup, deleted duplicate section

* Minor cleanup

* Process inbox messages using AJAX

* - removed import on INSTALLED_SCHEMES since its usage was removed on this commit  (aff43c0).
- limiting the version of celery to less then the 4.0 release, due to django-celery not having a release in over a year which would include the updates in master to support the new version of celery.  If you currently install Geonode it will install celery 4.0, but when attempting to use any django commands an ImportError occurs due to no module named timeutils.

* Split geoserver.helpers.set_attributes() (GeoNode#2699)

* Add conditional to geosites so signals are connected only if it is in INSTALLED_APPS.

* Added test for geonode.utils.set_attributes().

* Split helpers.set_attributes() into 2 functions.

* Renamed set_attributes() to set_attributes_from_geoserver().

* Moved really_set_attributes(), to geonode.utils and renamed to set_attributes().

* Removed unnecessary variable 'Attribute' from utils.

* Added check for layer/field pairs not in attribute_stats.

* flake8 formatting fixes.

* require celery version to be at least 3.1.18

When using `pip install -r requirements.txt` I was finding that my Celery
worker was not booting properly.

```
...
File "/env/lib/python2.7/site-packages/django/core/management/base.py", line 534, in check
    self.stderr.write(msg, lambda x: x)
TypeError: function takes exactly 1 argument (2 given)
```

This is an incompatibility between Celery 3.1.17 and Django 1.8.
celery/celery#2536

Fix: upgrade to Celery 3.1.18, which supports Django 1.8.
http://docs.celeryproject.org/en/latest/history/changelog-3.1.html#version-3-1-18

* The standalone TEMPLATE_* settings were deprecated in Django 1.8 (GeoNode#2711)

-  The standalone TEMPLATE_* settings were deprecated in Django 1.8 and the TEMPLATES dictionary takes precedence. You must put the values of the following settings into your default TEMPLATES dict: TEMPLATE_DIRS, TEMPLATE_CONTEXT_PROCESSORS, TEMPLATE_DEBUG. https://docs.djangoproject.com/en/1.8/topics/templates/#support-for-template-engines
- historic pep8 fixes

* Set SITEURL to server's host name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants