Skip to content
This repository has been archived by the owner on Dec 18, 2023. It is now read-only.

Made password configurable for tranSMART database. #41

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tgymnich
Copy link
Contributor

@tgymnich tgymnich commented Oct 26, 2019

Since the tranSMART Database will be exposed by port forwarding it makes sense to make the password user configurable.

@phozzy
Copy link
Contributor

phozzy commented Oct 27, 2019

@TG908 Hi! Add this information to README.

@phozzy
Copy link
Contributor

phozzy commented Oct 27, 2019

@ewelinagr will this require a change for entrypoint script of transmart container image?

@tgymnich
Copy link
Contributor Author

It looks like the password is set in here but never used.

@ewelinagr
Copy link
Member

Indeed, first we need to change transmart-api-server entrypoint to make this working. Here is a link to track the issue on transmart-api-server side: thehyve/transmart-core#472.

@tgymnich
Copy link
Contributor Author

I've made the necessary changes: thehyve/transmart-core#473

@tgymnich
Copy link
Contributor Author

any updates on this?

Copy link
Contributor

@gijskant gijskant left a comment

Choose a reason for hiding this comment

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

I agree that the password should be configurable. Please rename the variable to BIOMART_USER_PASSWORD to be consistent with the TranSMART database user/schema names.

@gijskant
Copy link
Contributor

Just a general remark: this user has super user rights on the database. So, it is better to not expose the database port by default, but only accessible for administrators, e.g., not exposing the port to outside the host machine, but only through an SSH tunnel.

docker-compose.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@gijskant gijskant left a comment

Choose a reason for hiding this comment

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

Could you squash your commits into a single one?

@tgymnich
Copy link
Contributor Author

done

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants