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

add missing settings in settings.py #4281

Merged
merged 10 commits into from
Mar 14, 2019
Merged

add missing settings in settings.py #4281

merged 10 commits into from
Mar 14, 2019

Conversation

ahmednoureldeen
Copy link
Contributor

update settings.py with settings defined in local_settings.py.geoserver.sample
to be able to run with default settings as local_settings.
updated settings related to site host and port and geoserver port and location.

@ahmednoureldeen
Copy link
Contributor Author

@afabiani I think we shouldn't check against hard-coded url as below

                if link['scheme'] == 'OGC:WMS':
                    self.assertEqual(
                        link['url'],
                        'http://localhost:8000/gs/ows',
                        'Expected a specific OGC:WMS URL')

for default GEOSERVER_PUBLIC_PORT and GEOSERVER_PUBLIC_HOST the url should be http://localhost:8080/geoserver/ows

geonode/local_settings.py.geoserver.sample Outdated Show resolved Hide resolved
'GEOSERVER_PUBLIC_PORT', 8080
)

_default_public_location = 'http://{}:{}/geoserver/'.format(GEOSERVER_PUBLIC_HOST, GEOSERVER_PUBLIC_PORT) if GEOSERVER_PUBLIC_PORT else 'http://{}/gs/'.format(GEOSERVER_PUBLIC_HOST)
Copy link
Member

Choose a reason for hiding this comment

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

This must be http://{}:{}/gs/ instead of http://{}:{}/geoserver/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the result url is not found for default values .. http://localhost:8080/gs
i think it should refere to geoserver url as the definition is for public port & host for geoserver meanwhile /gs is geonode url( act as a proxy for geoserver)

Copy link
Member

Choose a reason for hiding this comment

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

Yes, because of the default GEOSERVER_PUBLIC_PORT. It should be 8000 instead of 8080.

The policies are not to pass through the GeoNode proxy.

Copy link
Member

Choose a reason for hiding this comment

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

I have the impression that here we introduced a regression:

if _default_public_location ends always up with the form http://{}:{}/gs/ then the default result of GEOSERVER_WEB_UI_LOCATION is wrong because neither django urls nor jetty/tomcat have such a location /gs

@afabiani @giohappy what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

GEOSERVER_WEB_UI_LOCATION should be a variable with it's own value

Copy link
Member

Choose a reason for hiding this comment

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

Is it currently wrong indeed?

Copy link
Member

Choose a reason for hiding this comment

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

@francbartoli not if we are going to use GeoNode proxy

@afabiani afabiani merged commit b2a48ad into GeoNode:master Mar 14, 2019
@ahmednoureldeen ahmednoureldeen deleted the settings_fix branch March 19, 2019 12:48
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.

None yet

3 participants