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

Make Oracle SID connection param optional #4948

Merged
merged 1 commit into from May 3, 2017
Merged

Conversation

camsaul
Copy link
Member

@camsaul camsaul commented May 3, 2017

I don't know a lot about Oracle DBs but I have heard whispers in #4345 that specifying SID is not required, nor is it a BASIC NEED for all Oracle users. So let's make that param optional.

Includes tests

@camsaul camsaul added this to the 0.24.0 milestone May 3, 2017
@camsaul camsaul self-assigned this May 3, 2017
@camsaul camsaul merged commit e6be278 into master May 3, 2017
@camsaul camsaul deleted the make-oracle-sid-optional branch May 3, 2017 18:12
@alee792
Copy link

alee792 commented May 3, 2017

Just tested, but there seems to be some validation that's throwing an exception:

05-03 19:19:10 DEBUG metabase.middleware :: POST /api/database 400 (75 ms) (0 DB calls)
{:valid false,
:dbname "java.sql.SQLRecoverableException: IO Error: Invalid connection string format, a valid format is: "host:port:sid" ",
:message "java.sql.SQLRecoverableException: IO Error: Invalid connection string format, a valid format is: "host:port:sid" "}

Looks like it's still validating for an sid.

@camsaul
Copy link
Member Author

camsaul commented May 3, 2017

That validation isn't coming from Metabase, that's coming from the Oracle JDBC driver itself. Are you sure you don't have an SID? I don't think the driver would require one if it wasn't something everyone had

@camsaul
Copy link
Member Author

camsaul commented May 3, 2017

It sounds like maybe you can get your SID like this

SELECT sys_context('USERENV', 'SID') FROM DUAL;

@camsaul
Copy link
Member Author

camsaul commented May 3, 2017

It sounds like ORCL and XE are common default SIDs

@alee792
Copy link

alee792 commented May 3, 2017

That's what our DBA is telling me. I've tried a few different methods of pulling the SID, including the one you have posted above, to no avail. He did send over what he thinks the full JDBC string, so I'm going to test that independently right now to make sure he knows what he's talking about.

@camsaul
Copy link
Member Author

camsaul commented May 3, 2017

ok. Is the format like jdbc:oracle:thin:@//ipadress:portnumber/service_name? If so I can try tweaking a couple of things to construct the string like that (right now we're trying jdbc:oracle:thin:@//ipadress:portnumber?serviceName=service_name which sounds like it doesn't work)

@alee792
Copy link

alee792 commented May 3, 2017

Yep, that first one is what should work. I'm going to check it out in python to see if I can successfully connect.

@camsaul
Copy link
Member Author

camsaul commented May 3, 2017

Ok. Check out #4954. I pushed a branch that constructs the connection string that way instead of relying on params. Let me know if that works

@alee792
Copy link

alee792 commented May 3, 2017

image

Wooho, it's connected! Nothing's showing up in the data model right now, but the logs show it working through the schema. Thanks @camsaul!

@camsaul
Copy link
Member Author

camsaul commented May 3, 2017

@alee792 awesome!

@camsaul camsaul removed this from the 0.24.0 milestone May 3, 2017
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

2 participants