rucio icon indicating copy to clipboard operation
rucio copied to clipboard

Migration of SQLAlchemy queries to new query syntax (1.4 -> 2.0)

Open bari12 opened this issue 3 years ago • 9 comments

Motivation

With SQLAlchemy 2.0 the current sqlalchemy.orm.Session query syntax will become legacy and the new core query syntax should be used sqlalchemy.sql.expression.select. This is already available in version 1.4.

Modification

Over time, we need to migrate all queries to this new query syntax. This needs to be done module by module, with new PR only accepting the new query syntax.

bari12 avatar Feb 16 '22 14:02 bari12

@bari12 Which rucio modules are good ones to start with?

yuyiguo avatar Feb 17 '22 13:02 yuyiguo

Most of the SQLAlchemy interaction will be in the core

https://github.com/rucio/rucio/tree/master/lib/rucio/core

Best start with the smaller ones: account, distance, identity, ...

bari12 avatar Feb 17 '22 15:02 bari12

@bari12

@ericvaandering and I tried to generated RemovedIn20Warning. We enabled cms rucio int server with

SetEnv PYTHONWARNINGS always::DeprecationWarning
SetEnv SQLALCHEMY_WARN_20 1

https://github.com/yuyiguo/containers/blob/set-python-envs/server/rucio.conf.j2#L96 https://github.com/yuyiguo/CMSKubernetes/blob/set-python-envs/docker/rucio-server/Dockerfile#L17

I ran below commands:

rucio list-dataset-replicas cms:/BTag/Run2012A-v1/RAW#00e8f24c-993d-11e1-9605-003048caaace --deep
rucio list-rules "cms:/BTag/Run2012A-v1/RAW#00e8f24c-993d-11e1-9605-003048caaace"

In the server logs, I found:

k logs server-rucio-server-ff68c4d5b-7xp4d -c rucio-server | grep yuyi

[2022-04-04 23:59:54]	10.100.237.156	137.138.124.71	137.138.121.184	YkuGer33QIzLyfTp5u5@JwAAAb8	200	662	1508	43687	"GET /replicas/cms/%2FBTag%2FRun2012A-v1%2FRAW%2300e8f24c-993d-11e1-9605-003048caaace/datasets?deep=True HTTP/1.1"	"yuyi-/DC=ch/DC=cern/OU=Organic Units/OU=Users/CN=yuyi/CN=639751/CN=Yuyi Guo-unknown-243df3e747304cef9b369cf1efff8127"	"rucio-clients/1.27.2"	rucio::list-dataset-replicas

[2022-04-05 00:18:16]	10.100.237.156	137.138.124.71	137.138.121.184	YkuKyNQHC6LUJEs1VNnyZAAAAA0	200	634	1652	13665	"GET /dids/cms/%2FBTag%2FRun2012A-v1%2FRAW%2300e8f24c-993d-11e1-9605-003048caaace/rules HTTP/1.1"	"yuyi-/DC=ch/DC=cern/OU=Organic Units/OU=Users/CN=yuyi/CN=639751/CN=Yuyi Guo-unknown-243df3e747304cef9b369cf1efff8127"	"rucio-clients/1.27.2"	rucio::list-rules

But I did not found any RemovedIn20Warning . I wonder if the server was not configured to log warnings? or there are other logs I missed. BTW, there is no info in k logs server-rucio-server-ff68c4d5b-7xp4d -c httpd-error-log .

yuyiguo avatar Apr 05 '22 00:04 yuyiguo

I think this would rather be in the httpd-error-log (Or rucio-logs if this exists on the CMS instance); The one you show above is the access log.

But warnings I would assume it should log...

bari12 avatar Apr 05 '22 16:04 bari12

CMS does not have rucio-logs. I did not find RemovedIn20Warning in httpd-error-log, but it has something like below:

[Mon Apr 04 16:05:04.863228 2022] [suexec:notice] [pid 15:tid 140611666167936] AH01232: suEXEC mechanism enabled (wrapper: /usr/sbin/suexec)
[Mon Apr 04 16:05:04.887690 2022] [so:warn] [pid 15:tid 140611666167936] AH01574: module unique_id_module is already loaded, skipping
[Mon Apr 04 16:05:04.887719 2022] [so:warn] [pid 15:tid 140611666167936] AH01574: module wsgi_module is already loaded, skipping

[Mon Apr 04 16:05:04.887742 2022] [so:warn] [pid 15:tid 140611666167936] AH01574: module authn_core_module is already loaded, skipping
[Mon Apr 04 16:05:04.887752 2022] [so:warn] [pid 15:tid 140611666167936] AH01574: module cache_disk_module is already loaded, skipping
AH00558: httpd: Could not reliably determine the server's fully qualified domain name, using 10.100.237.156. Set the 'ServerName' directive globally to suppress this message
[Mon Apr 04 16:05:04.891442 2022] [lbmethod_heartbeat:notice] [pid 15:tid 140611666167936] AH02282: No slotmem from mod_heartmonitor

yuyiguo avatar Apr 05 '22 20:04 yuyiguo

Do you see any other Rucio debug/info message? The ones you show here are really only the apache specific ones, but not generated by Rucio.

bari12 avatar Apr 06 '22 14:04 bari12

No, I did not see any Rucio info.

yuyiguo avatar Apr 06 '22 20:04 yuyiguo

Is it possible STDERR from Rucio is going somewhere else?

ericvaandering avatar Apr 06 '22 20:04 ericvaandering

I created a pull request for db/sqla to migrate to SQLAlchemy 2.0 syntax rucio#5853 . This is the first time I have created a PR in Rucio development. I am unsure if the PR should be reviewed first, then I'll fix the existing conflicts and follow up with the reviewers' comments simultaneously. Please advise.

yuyiguo avatar Sep 01 '22 16:09 yuyiguo