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

Fixes for jdbcstore, jdbcconfig & jndi #699

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

tldev-de
Copy link

@tldev-de tldev-de commented Nov 5, 2024

Hi there,

this is my first contribution to this project, so please give me feedback if there is anything I can do better.

This commit fixes two problems, I experienced while using it for a larger deployment:

  1. jdbcstore does not get activated, since the changed configuration is written in jdbcstore.propertiesproperties instead of jdbcstore.properties
  2. jdbcconfig & jdbcstore cannot be used with jndi enabled, since the postgresql driver is not available

The first issue is solved by just fixing the filename. The second issue is fixed by adding the respective line to enable jndi in jdbcconfig / jdbcstore.

if you need more details to understand these problems, please let me know.

Hope this helps :)

Copy link
Collaborator

@NyakudyaA NyakudyaA left a comment

Choose a reason for hiding this comment

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

@tldev-de can we test this change, maybe provide a sample docker-compose with these settings and I will make a test in a separate PR

@@ -556,6 +556,13 @@ function setup_jdbc_db_config() {
else
envsubst < /build_data/jdbcconfig/jdbcconfig.properties > "${GEOSERVER_DATA_DIR}"/jdbcconfig/jdbcconfig.properties
fi

# Set jndiName if POSTGRES_JNDI is set to true
sed -i '/^jndiName=/d' "${GEOSERVER_DATA_DIR}"/jdbcconfig/jdbcconfig.properties
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't this line be inside the if statement?

fi

# Set jndiName if POSTGRES_JNDI is set to true
sed -i '/^jndiName=/d' "${GEOSERVER_DATA_DIR}"/jdbcstore/jdbcstore.properties
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as this as well

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.

2 participants