django
django copied to clipboard
Fixed #33817 -- Used python-oracledb and deprecate cx_Oracle.
cx_Oracle has a major new release under a new name and homepage python-oracledb.
The python-oracledb driver is a Python programming language extension module allowing Python programs to connect to Oracle Database. Python-oracledb is the new name for Oracle's popular cx_Oracle driver.
Tested on an Oracle Autonomous Database with TLSv1 connection.
Hello @petronny! Thank you for your contribution 💪
As it's your first contribution be sure to check out the patch review checklist.
If you're fixing a ticket from Trac make sure to set the "Has patch" flag and include a link to this PR in the ticket!
If you have any design or process questions then you can ask in the Django forum.
Welcome aboard ⛵️!
@petronny We should change tests/requirements/oracle.txt
.
buildbot, test on oracle.
@petronny Please push fixes in separate commits. It's hard to review after force-push.
Please push fixes in separate commits.
OK.
The last change only replaces oracledb.OBJECT
with oracle.DB_TYPE_OBJECT
. These two are same but the first one is deprecated.
This won't fix the failed test in oragis. I've created an issue https://github.com/oracle/python-oracledb/issues/43 for it on oracledb.
https://github.com/oracle/python-oracledb/issues/43#issuecomment-1206453289
According the reply of the oracledb group,
oracle.DB_TYPE_OBJECT
is not currently supported but may be supported in the future.
For now, we have to enable the thick mode of oracledb to pass this test, which needs the Oracle Client libraries. Are these libraries available in the oragis19 test?
Also, to enable the thick mode, a call to oracledb.init_oracle_client()
is needed.
But currently I don't know where to add this line just for this test.
For now, we have to enable the thick mode of oracledb to pass this test, which needs the Oracle Client libraries. Are these libraries available in the oragis19 test?
I'm guessing that since cx_Oracle was tested, then the libraries are available.
Overall, I recommend skipping this specific test when python-oracledb is in Thin mode (until the future time that the feature is supported in Thin mode). You could do two test runs: the first with Thin mode and the second with Thick mode.
I've updated the code to enable this test only for the thick mode of oracledb and cx_Oracle.
But I have no idea how to setup a test for the thick mode.
I've updated the code to enable this test only for the thick mode of oracledb and cx_Oracle.
But I have no idea how to setup a test for the thick mode.
Since you were able to run cx_Oracle so i assume you have the oracle instant client installed. So to switch from thin mode to thick mode in pyoracledb you need to call oracledb.init_oracle_client(lib_dir="path_to_instant_client"). You just need to call it once. You can call it either in the application or you can call after import oracledb in the following file django/db/backends/oracle/base.py.
You can call it either in the application or you can call after import oracledb in the following file django/db/backends/oracle/base.py.
It will force the mode to be thick if called in base.py. And I believe that most users should perfer the thin mode. So I perfer to call it just in the application.
But now we need some new jenkins workflows (or pipelines, idk) to test both the thin and thick modes, which are:
- oracle19, for thin mode and the basic fields.
- oragis19, for thin mode and the geometry fields.
- oracle19-thick, for thick mode and the basic fields.
- oragis19-thick , for thick mode and the geometry fields.
For the first two we can just use the existing oracle19 and oragis19 tests. I have no idea how to add the last two for jenkins and enable the thick mode in them.
@felixxm are you able to help with @petronny's question about enabling extra tests?
@felixxm are you able to help with @petronny's question about enabling extra tests?
IMO, we don't want to maintain multiple workflows for Oracle. We can decide to always use thick
mode as thin
mode is limited :thinking:. I should have some time to check this carefully in the coming weeks.
Well I wish there could be an option to still use the thin mode if the thick mode is set as the default since I don't need the geometry field.
@felixxm are you able to help with @petronny's question about enabling extra tests?
IMO, we don't want to maintain multiple workflows for Oracle. We can decide to always use
thick
mode asthin
mode is limited 🤔. I should have some time to check this carefully in the coming weeks.
@felixxm I think using default as thick would not server the benefit for which pyoracledb has been developed.In thin mode we dont require instant client library dependencies, its light weight as well faster. We are continuously working on expanding thin mode functionality. We can give option to users to choose between thin and thick mode using some parameters in the settings.py.
To get some progress on this PR, I can settle for using Thick mode until Thin mode has the Object support needed for all tests. Users (like us) will be able to keep testing Thin mode easily enough - particularly if this PR lands.
@felixxm Hi, I have tried to enable the thick mode in tests/dbshell/test_oracle.py
but I'm not sure if it will work. If it works then the PR is ready for review and the rest tests on the oracle database.
@petronny I had ran the test suite against your PR and have got some observations. The stats with you PR is: 15964 tests FAILED (failures=106, errors=41, skipped=1755, expected failures=6).
The stats with the current django 4.2dev version is: 15969 tests FAILED (failures=24, errors=3, skipped=1755, expected failures=6)
As we can see some new failures are coming with the PR which were not present earlier. Most of the failures are exactly same and repetative. AssertionError: 318 != 0 : Stream should be empty: actually contains '/Users/apple/oracle/django_mod1/django/django/db/backends/oracle/base.py:57
Test names : test_base_command_multiple_label (admin_scripts.tests.CommandTypes), test_testrunner_option (test_runner.tests.CustomTestRunnerOptionsCmdlineTests), etc.
Maybe some of the new changes is causing this failures.
@suraj-ora-2020 Thanks for the tests.
django/db/backends/oracle/base.py:57
gives a warning when oracledb is in the thin mode.
However the test wants no warning at all.
I've already called oracledb.init_oracle_client()
in tests/dbshell/test_oracle.py:15
. But maybe django/db/backends/oracle/base.py
is loaded earlier than that.
Could you have a try to just remove the warning at django/db/backends/oracle/base.py:57
to see if the tests pass?
If pass then I just need to figure out a new place to give the warning.
@suraj-ora-2020 Thanks for the tests.
django/db/backends/oracle/base.py:57
gives a warning when oracledb is in the thin mode. However the test wants no warning at all.I've already called
oracledb.init_oracle_client()
intests/dbshell/test_oracle.py:15
. But maybedjango/db/backends/oracle/base.py
is loaded earlier than that.Could you have a try to just remove the warning at
django/db/backends/oracle/base.py:57
to see if the tests pass? If pass then I just need to figure out a new place to give the warning.
@petronny The failures were due to the warnings. I tried removing the warnings and the tests were passing without failure.I guess the place for warnings need to be modified.
I guess the place for warnings need to be modified.
@suraj-ora-2020 I've moved that warning into DatabaseWrapper. Would you like to give it a new round of tests?
@petronny I can still see those errors coming from the modified place. AssertionError: 335 != 0 : Stream should be empty: actually contains '/Users/apple/oracle/django_mod1/django/django/db/backends/oracle/base.py:251
@suraj-ora-2020 Well, I've moved it to DatabaseWrapper.init_connection_state()
just before the cursor is created. Could you test if it works this time?
@suraj-ora-2020 Well, I've moved it to
DatabaseWrapper.init_connection_state()
just before the cursor is created. Could you test if it works this time?
@petronny Yes now it is working fine.
Great! Now let's wait for the review from @felixxm.
@felixxm do we have any update on review or merging of this PR.
do we have any update on review or merging of this PR.
If this is ready for re-review, the ticket should be updated to remove the "Needs ..." flags needs documentation, needs tests, patch needs improvement.
@jacobtylerwalls I see. Now it's done.
Hi @petronny. In django/db/backends/oracle/base.py file in line 275 can you please modify the Database.driver_mode.thin_mode to Database.is_thin_mode. https://python-oracledb.readthedocs.io/en/latest/api_manual/module.html#oracledb.is_thin_mode
@suraj-ora-2020 Updated.
Really looking forward to this feature, and big kudos to Oracle for the thin driver!