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

Update dbConnection.R - Require encryption #156

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sighanse
Copy link
Contributor

@sighanse sighanse commented Jun 5, 2024

Kan dere teste om dette slår på kryptering på mysql-tilkoblinger, og om det fungerer både mot gamle mysql5-databaser og nye mysql8-databaser? Jeg har ikke mulighet til å teste selv.

Require SSL connection to mysql database, but do not verify certificate
R/dbConnection.R Outdated
@@ -29,7 +29,9 @@ rapOpenDbConnection <- function(registryName, dbType = "mysql") {
host = conf$host,
user = conf$user,
password = conf$pass,
bigint = "integer"
bigint = "integer",
client.flag = CLIENT_SSL, # Request SSL connection
Copy link
Contributor

@arnfinn arnfinn Jun 5, 2024

Choose a reason for hiding this comment

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

CLIENT_SSL blir her tolket som et objekt eller variabel i R, som ikke finnes og vil derfor feile. Mulig client.flag=2048 vil fungere? Fant noe her: https://www.github.com/r-dbi/RMariaDB/issues/151

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Byttet den til 2048.

sighanse and others added 2 commits June 5, 2024 13:27
Constant CLIENT_SSL does not exist. Using 2048 instead.
@arnfinn
Copy link
Contributor

arnfinn commented Jun 5, 2024

Jeg tar med meg @kevinthon og tester det ut på test-serveren på fredag.

@sighanse
Copy link
Contributor Author

sighanse commented Jun 5, 2024

Lurer på om vi skulle ha tatt med en parameter på den mysqldump-greia også slik at export av databaser også fungerer.
La inn #160 også

@arnfinn
Copy link
Contributor

arnfinn commented Jun 6, 2024

Jeg har slått de to PR sammen i #164, også for å kunne teste i test-miljøet.

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