Do not makedsn for query_runner/oracle.py

[snickerjp]

What type of PR is this?

• Feature

Description

change for redash/query_runner/oracle.py
Do not makedsn for Autonomous Database or FAILOVER,LOAD_BALANCE

When type "_useservicename" in the host, self.configuration["servicename"] is used instead.

How is this tested?

• Unit tests (pytest, jest)
• E2E Tests (Cypress)
• Manually
• N/A

Related Tickets & Documents

Connecting to Oracle Database — cx_Oracle 8.3.0 documentation

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

[justinclift]

The idea here seems to be that when a hostname of _donotmakedsn is used, that instead triggers an alternative behaviour.

While it sounds like it would work, is this a good approach?

I'm not sure, but it feels to me like maybe the Oracle data source could have some different config option(s). Maybe something to directly choose a servicename instead of a hostname?

That's just my rough guessing anyway. 😄

[konnectr]

I think this is a change aimed at greater flexibility of connecting to the Oracle database. For my part, I would like to see a description of the possibility somewhere so that more people know about this possibility. And did not use 1 person :)

[snickerjp]

@justinclift

While it sounds like it would work, is this a good approach?

I thought the same thing.
However, with the current implementation in query_runner/oracle.py, I did not have the ability to refactor it to choose another method.

So, this time, I decided not to do makedsn.
I shared one option with you.

[snickerjp]

@konnectr
thanks.

added Comment in Settings menu.
I shared one option with you.

[snickerjp]

UI

[justinclift]

That string looks a bit ugly in the screenshot. I think the ( and ) are the problem.

Would you be ok to try this?

"Host: To use a DSN Service Name instead, use the text string `_useservicename` in the host name field."

I think that will look a bit better in the screenshot.

---

The backend-lint test is failing because of formatting problem.

If you have the Python dependencies installed, then make format should fix the problem:

https://github.com/getredash/redash/wiki/Local-development-setup#configuring-pre-commit

---

The frontend-e2e-tests test failure seems like a problem with the Yarn package repository. That will hopefully go away if we try again in a few minutes. 😄

[snickerjp]

Would you be ok to try this?

It certainly looks, not very desirable.
I understand. I will give it a try.

[snickerjp]

I tried to fix it.

• UI

[justinclift]

That is probably the best we can do for the UI for now. 😄

[snickerjp]

The backend-lint test is failing because of formatting problem.

I'll try this one too.
A moment of your time, please.

[justinclift]

The failure of the frontend-lint test this time seems to be a GitHub infrastructure problem.

GitHub has not been 100% reliable over the last few weeks. 😦

I'll wait a few minutes, then restart that test.

[codecov]

Codecov Report

Merging #6332 (e02aa77) into master (2b8aa5c) will decrease coverage by 0.01%.
Report is 4 commits behind head on master.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #6332      +/-   ##
==========================================
- Coverage   60.75%   60.74%   -0.01%     
==========================================
  Files         153      153              
  Lines       12511    12513       +2     
  Branches     1694     1695       +1     
==========================================
  Hits         7601     7601              
- Misses       4684     4686       +2     
  Partials      226      226              

Files Changed	Coverage Δ
redash/query_runner/oracle.py	31.25% <0.00%> (-0.67%)	⬇️

[justinclift]

Merged! Thanks for this @snickerjp, it's a good idea. 😄

[snickerjp]

Thanks for your help.