Opensearch #79

chaen · 2023-09-08T14:15:04Z

Example usage

# DIRACX_OS_DB_JOBPARAMETERSDB='{"hosts": "https://admin:admin@opensearch-cluster-master:9200", "use_ssl": true, "verify_certs": false}' HOME=/tmp/home ipython
import asyncio
import json
import os

from diracx.db.jobs.job_parameters import JobParametersDB


db = JobParametersDB(json.loads(os.environ["DIRACX_OS_DB_JOBPARAMETERSDB"]))

async with db.client_context():
    print(await db.search(None, None, None))
    job_id = 798811211
    doc = {
        'CEQueue': 'nordugrid-Condor-grid',
        'HostName': 'lapp-wn301.in2p3.fr',
        # 'JobID': job_id,
        "asad": "12345678"
    }
    await db.upsert(job_id, doc)
    await asyncio.sleep(2)
    print(await db.search(None, None, None))

TODOs:

Properly implement the search method
Add integration tests (once we have diracx integration tests, requires Add demo ci diracx-charts#34)
Restructure diracx.db to be split into diracx.db.os and diracx.db.sql?
Figure out how best to manage indicies and implement python -m diracx.db init-os
Create a BaseDB class with the commonalities between BaseSQLDB and BaseOSDB?
~~Rename client_context/engine_context to lifetime_function?~~ Probably not

chaen · 2023-09-15T14:25:24Z

src/diracx/db/__init__.py

-from .auth.db import AuthDB
-from .jobs.db import JobDB, JobLoggingDB
-from .sandbox_metadata.db import SandboxMetadataDB
+from .sql.auth.db import AuthDB


Should that stay here, or do we want to expose to the routers the sql or os ?
Since we started making the distinction, it's probably worth pursuing so a router would do from diracx.db.sql import AuthDB

Yeah I agree, especially as the behavour and API is different in each case.

…DB_TYPE

fstagni

To the cost of being a bit more verbose, I would expand everywhere "os" -> "opensearch", for clarity.
Also, as of now you added directories tests/db/opensearch and src/diracx/db/os, I would reconcile everything to opensearch.

fstagni · 2023-09-18T08:40:37Z

src/diracx/db/__main__.py

+    from diracx.db.os.utils import BaseOSDB
+
+    for db_name, db_url in BaseOSDB.available_urls().items():
+        logger.info("Initialising %s", db_name)


Suggested change

logger.info("Initialising %s", db_name)

logger.info(f"Initialising {db_name}")

fstagni · 2023-09-18T08:44:24Z

src/diracx/db/os/job_parameters.py

+        "HostName": {"type": "keyword"},
+        "GridCE": {"type": "keyword"},
+        "ModelName": {"type": "keyword"},
+        "Status": {"type": "keyword"},


I think Status should be removed: DIRACGrid/DIRAC#7191
And a few of these should be moved to ElasticPilotParameters: DIRACGrid/DIRAC#7164

Do you want to create this implementation anyway now for testing? In this case put some ToDO notes (the links above would suffice).

fstagni · 2023-09-18T08:45:28Z

src/diracx/db/os/utils.py

+            for entry_point in select_from_extension(group="diracx.db.os", name=db_name)
+        ]
+        if not db_classes:
+            raise NotImplementedError(f"Could not find any matches for {db_name=}")


Suggested change

raise NotImplementedError(f"Could not find any matches for {db_name=}")

raise NotImplementedError(f"Could not find any matches for {db_name}")

I think that's the intended behavior

In [1]: a=3 In [2]: f"{a=}" Out[2]: 'a=3'

fstagni · 2023-09-18T08:46:02Z

src/diracx/db/os/utils.py

+                try:
+                    conn_kwargs[db_name] = json.loads(os.environ[var_name])
+                except Exception:
+                    logger.error("Error loading connection parameters for %s", db_name)


Suggested change

logger.error("Error loading connection parameters for %s", db_name)

logger.error(f"Error loading connection parameters for {db_name}")```

fstagni · 2023-09-18T08:48:18Z

src/diracx/db/os/utils.py

+
+    async def __aenter__(self):
+        """This is entered on every request. It does nothing"""
+        assert self._client is None, "client_context hasn't been entered"


Suggested change

assert self._client is None, "client_context hasn't been entered"

assert self._client is None, "client_context has not been entered"

when it is about printing something, I would avoid any possibly-interpreted-as-special characters.

fstagni · 2023-09-18T08:49:52Z

src/diracx/db/sql/utils.py

@@ -107,7 +109,7 @@ def available_urls(cls) -> dict[str, str]:
                    else:
                        db_urls[db_name] = parse_obj_as(SqlalchemyDsn, db_url)
                except Exception:
-                    logger.error("Error loading URL %s for %s", db_name, db_url)
+                    logger.error("Error loading URL for %s", db_name)


Suggested change

logger.error("Error loading URL for %s", db_name)

logger.error(f"Error loading URL for {db_name}")

This is intentional, see #35 (comment)

fstagni · 2023-09-18T09:35:07Z

My review was completely ignored?

chaen · 2023-09-18T12:09:19Z

Sorry, I did not see it, we were discussing it on a shared screen and I did not refresh the page.
I agree with some of the comments, but I wouldn't replace OS to OpenSearch, it's just too long.

chaen force-pushed the opensearch branch from 27db5c9 to 1e67c02 Compare September 10, 2023 05:21

chrisburr force-pushed the opensearch branch 6 times, most recently from a5b80e6 to 264c01f Compare September 15, 2023 14:15

chrisburr approved these changes Sep 15, 2023

View reviewed changes

chrisburr force-pushed the opensearch branch from 264c01f to c6d42fa Compare September 15, 2023 14:16

chaen commented Sep 15, 2023

View reviewed changes

chrisburr force-pushed the opensearch branch from cf5e33c to 81270a6 Compare September 18, 2023 06:27

chaen and others added 10 commits September 18, 2023 08:40

Add opensearch dependencies

9f15924

refactor utils.BaseDB to sql_utils.BaseSQLDB

c42afb0

Don't log DB URL

c1bd65b

tmp start adding base opensearch

7e0e96f

Add BaseOSDB

094ee79

Split diracx.db into diracx.db.sql and diracx.db.os

c363685

Move imports of database implementations from diracx.db to diracx.db.…

c09c980

…DB_TYPE

Add OpenSearch connection tests

7b7605c

Support managing index templates with DiracX

9ebc714

Add workaround for broken integration tests

4218b9d

chrisburr force-pushed the opensearch branch from 00ff164 to 4218b9d Compare September 18, 2023 06:40

fstagni reviewed Sep 18, 2023

View reviewed changes

chaen merged commit 70ebe49 into DIRACGrid:main Sep 18, 2023
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Opensearch #79

Opensearch #79

chaen commented Sep 8, 2023 •

edited by chrisburr

Loading

chaen Sep 15, 2023

chrisburr Sep 15, 2023

fstagni left a comment

fstagni Sep 18, 2023

fstagni Sep 18, 2023

fstagni Sep 18, 2023

chaen Sep 18, 2023

fstagni Sep 18, 2023

fstagni Sep 18, 2023

fstagni Sep 18, 2023

chrisburr Sep 21, 2023

fstagni commented Sep 18, 2023

chaen commented Sep 18, 2023

	logger.info("Initialising %s", db_name)
	logger.info(f"Initialising {db_name}")

	raise NotImplementedError(f"Could not find any matches for {db_name=}")
	raise NotImplementedError(f"Could not find any matches for {db_name}")

	logger.error("Error loading connection parameters for %s", db_name)
	logger.error(f"Error loading connection parameters for {db_name}")```

	assert self._client is None, "client_context hasn't been entered"
	assert self._client is None, "client_context has not been entered"

	logger.error("Error loading URL for %s", db_name)
	logger.error(f"Error loading URL for {db_name}")

Opensearch #79

Opensearch #79

Conversation

chaen commented Sep 8, 2023 • edited by chrisburr Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fstagni left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fstagni commented Sep 18, 2023

chaen commented Sep 18, 2023

chaen commented Sep 8, 2023 •

edited by chrisburr

Loading