From 829d32154e0e4355b1ff8b1a564d1f792f97d957 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florentin=20D=C3=B6rre?= Date: Tue, 16 Jul 2024 11:06:44 +0200 Subject: [PATCH 1/3] Skip listing sessions over paused dbs during delete lookup --- .../session/dedicated_sessions.py | 9 +++- .../tests/unit/test_dedicated_sessions.py | 48 ++++++++++++++++--- 2 files changed, 48 insertions(+), 9 deletions(-) diff --git a/graphdatascience/session/dedicated_sessions.py b/graphdatascience/session/dedicated_sessions.py index d27124616..466c6fcac 100644 --- a/graphdatascience/session/dedicated_sessions.py +++ b/graphdatascience/session/dedicated_sessions.py @@ -51,7 +51,6 @@ def get_or_create( # hashing the password to avoid storing the actual db password in Aura password = hashlib.sha256(db_connection.password.encode()).hexdigest() - # TODO configure session size (and check existing_session has same size) if existing_session: self._check_expiry_date(existing_session) self._check_memory_configuration(existing_session, memory.value) @@ -106,7 +105,13 @@ def list(self) -> List[SessionInfo]: return [SessionInfo.from_session_details(i) for i in sessions] def _find_existing_session(self, session_name: str, dbid: str) -> Optional[SessionDetails]: - matched_sessions = [s for s in self._aura_api.list_sessions(dbid) if s.name == session_name] + matched_sessions: List[SessionDetails] = [] + try: + matched_sessions = [s for s in self._aura_api.list_sessions(dbid) if s.name == session_name] + except HTTPError as e: + # ignore 404 errors when listing sessions as it could mean paused sessions or deleted sessions + if e.response.status_code != 404: + raise e if len(matched_sessions) == 0: return None diff --git a/graphdatascience/tests/unit/test_dedicated_sessions.py b/graphdatascience/tests/unit/test_dedicated_sessions.py index 77a512c00..68888dad3 100644 --- a/graphdatascience/tests/unit/test_dedicated_sessions.py +++ b/graphdatascience/tests/unit/test_dedicated_sessions.py @@ -69,6 +69,15 @@ def add_session(self, session: SessionDetails) -> None: self._sessions[session.id] = session + # aura behaviour of paused instances not being in an orchestra + def _mimic_paused_db_behaviour(self, dbid: str) -> None: + db = self.list_instance(dbid) + if db and db.status == "paused": + response = Response() + response.status_code = 404 + response._content = b"database not found" + raise HTTPError(request=None, response=response) + def create_instance( self, name: str, memory: SessionMemoryValue, cloud_provider: str, region: str ) -> InstanceCreateDetails: @@ -104,13 +113,7 @@ def delete_instance(self, instance_id: str) -> InstanceSpecificDetails: return self._instances.pop(instance_id) def list_sessions(self, dbid: str) -> List[SessionDetails]: - # mimic aura behaviour of paused instances not being in an orchestra - db = self.list_instance(dbid) - if db and db.status == "paused": - response = Response() - response.status_code = 404 - response._content = b"database not found" - raise HTTPError(request=None, response=response) + self._mimic_paused_db_behaviour(dbid) return [v for _, v in self._sessions.items() if v.instance_id == dbid] @@ -118,6 +121,8 @@ def list_instances(self) -> List[InstanceDetails]: return [v for _, v in self._instances.items()] def list_session(self, session_id: str, dbid: str) -> Optional[SessionDetails]: + self._mimic_paused_db_behaviour(dbid) + matched_instance = self._sessions.get(session_id, None) if matched_instance: @@ -399,6 +404,35 @@ def test_delete_nonunique_session(aura_api: AuraApi) -> None: assert len(sessions.list()) == 2 +def test_delete_session_paused_instance(aura_api: AuraApi) -> None: + fake_aura_api = cast(FakeAuraApi, aura_api) + + fake_aura_api.id_counter += 1 + paused_db = InstanceSpecificDetails( + id="4242", + status="paused", + connection_url="foo.bar", + memory=SessionMemory.m_16GB.value, + type="", + region="dresden", + name="paused-db", + tenant_id=fake_aura_api._tenant_id, + cloud_provider="aws", + ) + fake_aura_api._instances[paused_db.id] = paused_db + + session = aura_api.create_session( + name="gds-session-my-session-name", + dbid=paused_db.id, + pwd="some_pwd", + memory=SessionMemory.m_8GB.value, + ) + sessions = DedicatedSessions(aura_api) + + # cannot delete session running against a paused instance + assert not sessions.delete(session.name) + + def test_create_waiting_forever() -> None: aura_api = FakeAuraApi(status_after_creating="updating") _setup_db_instance(aura_api) From 8e6efabedafd977fdef4e93fc897e6f7887145e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florentin=20D=C3=B6rre?= Date: Tue, 16 Jul 2024 11:09:46 +0200 Subject: [PATCH 2/3] Delete session by id on gds.delete --- graphdatascience/session/dedicated_sessions.py | 8 +++++--- graphdatascience/tests/unit/test_dedicated_sessions.py | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/graphdatascience/session/dedicated_sessions.py b/graphdatascience/session/dedicated_sessions.py index 466c6fcac..714c7b33e 100644 --- a/graphdatascience/session/dedicated_sessions.py +++ b/graphdatascience/session/dedicated_sessions.py @@ -70,7 +70,7 @@ def get_or_create( ) return self._construct_client( - session_name=session_name, session_connection=session_connection, db_connection=db_connection + session_id=session_id, session_connection=session_connection, db_connection=db_connection ) def delete(self, session_name: str, dbid: Optional[str] = None) -> bool: @@ -137,12 +137,14 @@ def _create_session( return create_details def _construct_client( - self, session_name: str, session_connection: DbmsConnectionInfo, db_connection: DbmsConnectionInfo + self, session_id: str, session_connection: DbmsConnectionInfo, db_connection: DbmsConnectionInfo ) -> AuraGraphDataScience: return AuraGraphDataScience( gds_session_connection_info=session_connection, aura_db_connection_info=db_connection, - delete_fn=lambda: self.delete(session_name, dbid=AuraApi.extract_id(db_connection.uri)), + delete_fn=lambda: self._aura_api.delete_session( + session_id=session_id, dbid=AuraApi.extract_id(db_connection.uri) + ), ) def _check_expiry_date(self, session: SessionDetails) -> None: diff --git a/graphdatascience/tests/unit/test_dedicated_sessions.py b/graphdatascience/tests/unit/test_dedicated_sessions.py index 68888dad3..aa76b18b4 100644 --- a/graphdatascience/tests/unit/test_dedicated_sessions.py +++ b/graphdatascience/tests/unit/test_dedicated_sessions.py @@ -244,7 +244,7 @@ def test_create_session(mocker: MockerFixture, aura_api: AuraApi) -> None: "session_connection": DbmsConnectionInfo( uri="neo4j+s://foo.bar", username="neo4j", password=HASHED_DB_PASSWORD ), - "session_name": "my-session", + "session_id": "ffff0-ffff1", } assert [i.name for i in sessions.list()] == ["my-session"] @@ -274,7 +274,7 @@ def test_get_or_create(mocker: MockerFixture, aura_api: AuraApi) -> None: "session_connection": DbmsConnectionInfo( uri="neo4j+s://foo.bar", username="neo4j", password=HASHED_DB_PASSWORD ), - "session_name": "my-session", + "session_id": "ffff0-ffff1", } assert gds_args1 == gds_args2 From 0273a68f9eb03eeb220698654989e2588051d1c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florentin=20D=C3=B6rre?= Date: Tue, 16 Jul 2024 11:14:23 +0200 Subject: [PATCH 3/3] Use session.delete in notebook example --- .../ROOT/pages/tutorials/gds-sessions.adoc | 26 ++++++++++--------- examples/gds-sessions.ipynb | 26 ++++++++++--------- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/doc/modules/ROOT/pages/tutorials/gds-sessions.adoc b/doc/modules/ROOT/pages/tutorials/gds-sessions.adoc index 4392f4139..edd944439 100644 --- a/doc/modules/ROOT/pages/tutorials/gds-sessions.adoc +++ b/doc/modules/ROOT/pages/tutorials/gds-sessions.adoc @@ -124,12 +124,12 @@ data_query = """ (brie:Person {name: 'Brie', age: 31, experience: 6, hipster: 0}), (elsa:Person {name: 'Elsa', age: 65, experience: 23, hipster: 1}), (john:Person {name: 'John', age: 4, experience: 100, hipster: 0}), - + (apple:Fruit {name: 'Apple', tropical: 0, sourness: 0.3, sweetness: 0.6}), (banana:Fruit {name: 'Banana', tropical: 1, sourness: 0.1, sweetness: 0.9}), (mango:Fruit {name: 'Mango', tropical: 1, sourness: 0.3, sweetness: 1.0}), (plum:Fruit {name: 'Plum', tropical: 0, sourness: 0.5, sweetness: 0.8}) - + CREATE (dan)-[:LIKES]->(apple), (annie)-[:LIKES]->(banana), @@ -138,7 +138,7 @@ data_query = """ (brie)-[:LIKES]->(banana), (elsa)-[:LIKES]->(plum), (john)-[:LIKES]->(plum), - + (dan)-[:KNOWS]->(annie), (dan)-[:KNOWS]->(matt), (annie)-[:KNOWS]->(matt), @@ -179,16 +179,16 @@ G, result = gds.graph.project( CALL { MATCH (p1:Person) OPTIONAL MATCH (p1)-[r:KNOWS]->(p2:Person) - RETURN - p1 AS source, r AS rel, p2 AS target, - p1 {.age, .experience, .hipster } AS sourceNodeProperties, + RETURN + p1 AS source, r AS rel, p2 AS target, + p1 {.age, .experience, .hipster } AS sourceNodeProperties, p2 {.age, .experience, .hipster } AS targetNodeProperties UNION MATCH (f:Fruit) OPTIONAL MATCH (f)<-[r:LIKES]-(p:Person) - RETURN - p AS source, r AS rel, f AS target, - p {.age, .experience, .hipster } AS sourceNodeProperties, + RETURN + p AS source, r AS rel, f AS target, + p {.age, .experience, .hipster } AS sourceNodeProperties, f { .tropical, .sourness, .sweetness } AS targetNodeProperties } RETURN gds.graph.project.remote(source, target, { @@ -265,8 +265,8 @@ instance. ---- gds.run_cypher( """ - MATCH (p:Person) - RETURN p.name, p.pagerank AS rank, p.louvain + MATCH (p:Person) + RETURN p.name, p.pagerank AS rank, p.louvain ORDER BY rank DESC """ ) @@ -284,7 +284,9 @@ stop incurring costs. [source, python, role=no-test] ---- -sessions.delete("people-and-fruits") +gds.delete() + +# or sessions.delete("people-and-fruits") ---- [source, python, role=no-test] diff --git a/examples/gds-sessions.ipynb b/examples/gds-sessions.ipynb index b84b41887..79a3c3ae8 100644 --- a/examples/gds-sessions.ipynb +++ b/examples/gds-sessions.ipynb @@ -181,12 +181,12 @@ " (brie:Person {name: 'Brie', age: 31, experience: 6, hipster: 0}),\n", " (elsa:Person {name: 'Elsa', age: 65, experience: 23, hipster: 1}),\n", " (john:Person {name: 'John', age: 4, experience: 100, hipster: 0}),\n", - " \n", + "\n", " (apple:Fruit {name: 'Apple', tropical: 0, sourness: 0.3, sweetness: 0.6}),\n", " (banana:Fruit {name: 'Banana', tropical: 1, sourness: 0.1, sweetness: 0.9}),\n", " (mango:Fruit {name: 'Mango', tropical: 1, sourness: 0.3, sweetness: 1.0}),\n", " (plum:Fruit {name: 'Plum', tropical: 0, sourness: 0.5, sweetness: 0.8})\n", - " \n", + "\n", " CREATE\n", " (dan)-[:LIKES]->(apple),\n", " (annie)-[:LIKES]->(banana),\n", @@ -195,7 +195,7 @@ " (brie)-[:LIKES]->(banana),\n", " (elsa)-[:LIKES]->(plum),\n", " (john)-[:LIKES]->(plum),\n", - " \n", + "\n", " (dan)-[:KNOWS]->(annie),\n", " (dan)-[:KNOWS]->(matt),\n", " (annie)-[:KNOWS]->(matt),\n", @@ -242,16 +242,16 @@ " CALL {\n", " MATCH (p1:Person)\n", " OPTIONAL MATCH (p1)-[r:KNOWS]->(p2:Person)\n", - " RETURN \n", - " p1 AS source, r AS rel, p2 AS target, \n", - " p1 {.age, .experience, .hipster } AS sourceNodeProperties, \n", + " RETURN\n", + " p1 AS source, r AS rel, p2 AS target,\n", + " p1 {.age, .experience, .hipster } AS sourceNodeProperties,\n", " p2 {.age, .experience, .hipster } AS targetNodeProperties\n", " UNION\n", " MATCH (f:Fruit)\n", " OPTIONAL MATCH (f)<-[r:LIKES]-(p:Person)\n", - " RETURN \n", - " p AS source, r AS rel, f AS target, \n", - " p {.age, .experience, .hipster } AS sourceNodeProperties, \n", + " RETURN\n", + " p AS source, r AS rel, f AS target,\n", + " p {.age, .experience, .hipster } AS sourceNodeProperties,\n", " f { .tropical, .sourness, .sweetness } AS targetNodeProperties\n", " }\n", " RETURN gds.graph.project.remote(source, target, {\n", @@ -361,8 +361,8 @@ "source": [ "gds.run_cypher(\n", " \"\"\"\n", - " MATCH (p:Person) \n", - " RETURN p.name, p.pagerank AS rank, p.louvain \n", + " MATCH (p:Person)\n", + " RETURN p.name, p.pagerank AS rank, p.louvain\n", " ORDER BY rank DESC\n", " \"\"\"\n", ")" @@ -391,7 +391,9 @@ }, "outputs": [], "source": [ - "sessions.delete(\"people-and-fruits\")" + "gds.delete()\n", + "\n", + "# or sessions.delete(\"people-and-fruits\")" ] }, {