Skip to content

Fix deleting session by name against paused instances #685

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 16, 2024
Merged

Fix deleting session by name against paused instances #685

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
829d321
Skip listing sessions over paused dbs during delete lookup
FlorentinD Jul 16, 2024
8e6efab
Delete session by id on gds.delete
FlorentinD Jul 16, 2024
0273a68
Use session.delete in notebook example
FlorentinD Jul 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 14 additions & 12 deletions doc/modules/ROOT/pages/tutorials/gds-sessions.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -138,7 +138,7 @@ data_query = """
(brie)-[:LIKES]->(banana),
(elsa)-[:LIKES]->(plum),
(john)-[:LIKES]->(plum),

(dan)-[:KNOWS]->(annie),
(dan)-[:KNOWS]->(matt),
(annie)-[:KNOWS]->(matt),
Expand Down Expand Up @@ -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, {
Expand Down Expand Up @@ -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
"""
)
Expand All @@ -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]
Expand Down
26 changes: 14 additions & 12 deletions examples/gds-sessions.ipynb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
")"
Expand Down Expand Up @@ -391,7 +391,9 @@
},
"outputs": [],
"source": [
"sessions.delete(\"people-and-fruits\")"
"gds.delete()\n",
"\n",
"# or sessions.delete(\"people-and-fruits\")"
]
},
{
Expand Down
17 changes: 12 additions & 5 deletions graphdatascience/session/dedicated_sessions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -71,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:
Expand Down Expand Up @@ -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
Expand All @@ -132,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:
Expand Down
52 changes: 43 additions & 9 deletions graphdatascience/tests/unit/test_dedicated_sessions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -104,20 +113,16 @@ 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]

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:
Expand Down Expand Up @@ -239,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"]

Expand Down Expand Up @@ -269,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

Expand Down Expand Up @@ -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)
Expand Down