Enable python-oracledb ThickMode while keeping sqlalchemy on 1.4 #12451
Conversation
|
test this please |
|
I've injected one validation workflow in vocms0290, which is not having the old No errors in the logs were observed either. |
|
Just an update about the Jenkins errors which we observe. They are all following from the missing OS level Oracle driver libraries. The underlying error is:
This comes from the fact that we decided to go with the
And more details about this error is given in the And indeed fetching this information from the database gives the result: Which basically tells us that ORacle Native Network Encryption and Data Integrity (NNE) is enabled on the server side. so we must resort to one of the two solutions suggested in the documentation: """
""" I chose to go with the p.s. The fact that we are using SQLAlchemy on top of the native database libraries did not make things any simpler :) [1] [2] |
|
test this please |
|
test this please |
|
test this please |
|
Jenkins results:
|
|
Jenkins is finally fixed... After loosing one day to figure out how to configure the failing The so failing unit tests are another story though. I will look into them now, but I am not sure I can do much. |
|
test this please |
|
As mentioned yesterday, None of these failing unit tests are actually related to the code changes here. @amaltaro @anpicci I can say this PR is in final state. Feel free to leave your review comments so that we can merge. In the meantime I'll create a local build and a tag one docker image in one of my agents only (not uploading it to CERN registry) so that I can deploy and confirm that this change does not affect our MariaDB containers. |
|
Jenkins results:
|
There was a problem hiding this comment.
@todor-ivanov I left a comment along the code for your consideration.
In addition,
- can you please update the initial description of the PR with all relevant changes to integrate this Oracle library (CMSKubernetes, Jenkins, etc)?
- the PR title suggests a change in SQLAlchemy library, but I see no changes to the requirements.txt file. Does it mean the version didn't really change?
| import oracledb | ||
| oracledb.version = "8.3.0" | ||
| sys.modules["cx_Oracle"] = oracledb | ||
| oracledb.init_oracle_client() |
There was a problem hiding this comment.
Can you please add a note or a pointer to the PR explaining why we need to make these tweaks?
It sounds strange to me that we need to define a cx_Oracle module in our code, considering that it has been deprecated/replaced in 2022 by python-oracledb.
In addition, I think this should only be applied if we identify that the dialect is oracle. In other words, this isn't really needed for MariaDB backend agents.
There was a problem hiding this comment.
hi @amaltaro I'll reply to this comment not inline but in the main thread, because it will be long: #12451 (comment)
There was a problem hiding this comment.
Maybe add something like "Ensure SQLAlchemy is able to find the oracledb python libraries.\nFurther details at: "
It will help if anyone needs to look into this code in the future.
|
Hi @todor-ivanov, thank you for your work! May you fix the failed unit tests? |
|
Hi @amaltaro @anpicci , I have also injected a workflow to confirm it explicitly: https://cmsweb-testbed.cern.ch/reqmgr2/fetch?rid=tivanov_SC_Parent_LumiMask_cx_oracle_Test_v3_251007_082926_9592 I will report the results from it later. |
|
Hi @amaltaro in reply to your comment inline: https://github.com/dmwm/WMCore/pull/12451/files#r2401650002
Well, this even though at a first glance sounds a legit request, it is difficult to implement on a cost which I am not sure, we are willing to pay at this stage. What I am talking about - The module which is actually creating the DB connection is indeed: https://github.com/dmwm/WMCore/blob/master/src/python/WMCore/Database/DBFactory.py (where the current dialect is not known) but not in the https://github.com/dmwm/WMCore/blob/master/src/python/WMCore/WMInit.py which is calling the former and knows the exact dialect for the current agent: WMCore/src/python/WMCore/WMInit.py Lines 118 to 129 in 824f926 Of course at DBFactory it can be taken from the thread itslef... because it is written there from WMInit: WMCore/src/python/WMCore/WMInit.py Line 131 in 824f926 But I, considered it more dangerous because |
|
About: #12451 (review)
They are already there. I've put them in this description as I was developing them.
The PR's does not suggest anything. It is explicit on the sqlAlchemy version - v1.4. Which is the version we are currently using. |
|
@todor-ivanov , thank you for the thorough investigation about the ties between oracle and the SQLAlchemy version. We will not upgrade SQLAlchemy, so I strongly suggest to stick with the recommendations from oracle about how to interface the new python library with SQLAlchemy 1.4. Efforts should only deliver a working python-oracledb + SQLAlchemy 1.4 configuration, as we did not target an SQLAlchemy update when making this issue part of the Q3 development plan. About the dialect, if what has been already provided is enough to consistently propagate the database flavor all over the stack, please do not take any other action. About the unit tests, we can proceed as agreed in private on MM. |
|
Thank you for the follow up, Todor.
Mentioning SQLAlchemy in the title looks confusing to me and it lead me to think that it was being changed as well. |
|
By the way, dmwm/CMSKubernetes#1663 only shows changes to the |
todor-ivanov
changed the title
|
thanks @amaltaro
I did not see anywhere a claim that
I changed the title to avoid confusion. |
It was related only to fixing our dev image which we run at Jenkins. Once we converge on which Oracle instant client we moe on with I'll add also this PR to the description link as well. |
|
test this please |
Add OracleDB initializtion comment
|
Jenkins results:
|
todor-ivanov
force-pushed
the
feature_WMAgent_Migrate_cx_Oracle_fix-12395
branch
from
e0e1d37 to
43936c0
Compare
|
@todor-ivanov please investigate the 3 new failing unit tests before merging this. |
|
test this please |
|
Jenkins results:
|
|
Hi @amaltaro those seemed transient. And the Jenkins error is gone now |
|
test this please |
|
Jenkins results:
|
|
@todor-ivanov the unstable unittests cleared out, but these 3 are still persisting and I think it is important to understand and fix what is causing their failure: |
|
Hi @amaltaro I changed nothing in my Jenkins setup related to the latest failures. I just requested a fresh test through the GH actions. As of the failing unit tests, none of them seem to be related to the current code change. |
|
Nevertheless, lets look into each one of them individually.
With the tests you refer to in the top 3 from the list. ( I have no idea if that matters actually)
Which is far from enough, so lets do a manual run and add some printouts: So, nothing to do with the current development
This time lets run it individually (only the failing one reported: This one even runs successfully on my setup!
Again, lets run it individually (only the failing one reported - Successful again! While if we really want to see what is breaking for this unittest file, it is not the
|
|
test this please |
|
Jenkins results:
|
|
test this please |
|
And now the same 3 unittests are also failing in the baseline tests, as can be seen in this report: I don't think any of them are related to these changes, but the |
|
test this please |
|
Jenkins results:
|
4 participants
Fixes #12395
Status
Ready
Description
With the current PR we enable python-oracledb ThickMode + sqlalchemy 1.4
Is it backward compatible (if not, which system it affects?)
Yes
Related PRs
libaiolibrary issue at image build time rather on runtime: #CMSKubernetes/1667 #CMSKubernetes/1672wmagent-baseandwmagentimages to use the latestoracleimage uploaded to CERN Registry && remove thelibaioruntime workaround: #CMSKubernetes/1670External dependencies / deployment changes
This PR introduces a new OS level dependency, which must be enforced also to our
wmcore-devimages which we run within our Jenkins setup. The new image is switched to at the configuration of this Jenkins job: https://cmssdt.cern.ch/dmwm-jenkins/job/WMCore-PR-Test/