-
Notifications
You must be signed in to change notification settings - Fork 9
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 geoserver printing host customiization #225
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AlexGacon for me its very unclear where all the configuration inside of the charts/geonode/templates/geoserver/geoserver-printing-config-yaml.yaml
files comes from. Are this default values from the GeoServer or did you gather them by yourself?
#edit: ok after reading the related issue I fully overview the problem. Maybe you could at least add a header to the yaml file describing where the content is comming from and that its served or controlled by geonode-k8s helm chart. So people manually straving inside the geoserver container will know what they are looking at.
charts/geonode/templates/geoserver/geoserver-printing-config-yaml.yaml
Outdated
Show resolved
Hide resolved
brokenUrlPlaceholder: 'default' | ||
# brokenUrlPlaceholder: 'throw' | ||
|
||
#proxyBaseUrl: http://geoserver:8080/geoserver/pdf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove commented code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
||
#=========================================================================== | ||
# the list of allowed ips | ||
#=========================================================================== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you add some documentation here where this list is coming from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation added and I have moved the custom part at the end of the list, with additional comments.
@mwallschlaeger is the removal of notifier.xml ok for you? |
@AlexGacon yes removing of the xml is fine, as its served via geoserver docker image |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine to me now in total.
Description
This PR completes the configuration of the printing service of GeoServer to works with the URL of GeoNode and adds the possibility to define additional hosts in the values.yaml file.
Type of Change
Please select the relevant option:
Related Issue
If there is an existing issue related to this pull request, please reference it here.
closes #223
Checklist
Please ensure that your pull request meets the following requirements:
Additional Notes
Any additional information or context regarding the pull request can be provided here.
Thank you for creating this pull request