mithril-security / blindai

Confidential AI deployment with secure enclaves :lock:
https://www.mithrilsecurity.io/
Apache License 2.0
500 stars 35 forks source link

feat(client)!: Add Python context support aka with statement #78

Closed clauverjat closed 2 years ago

clauverjat commented 2 years ago

Description

The current API can result in the user forgetting to close the connection to the server. This PR introduces support for Python context to the BlindAiClient. It also changes the client interface. The BlindAiClient class has been removed, in favor of a new BlindAiConnection class that can be created via the blindai.client.connect function.

Client usage before :

client = BlindAiClient()
client.connect_server(addr="localhost", simulation=True)
# do something with client...
client.close_connection()

Client usage after the change :

with blindai.client.connect(addr="localhost", simulation=True) as client:
  # do something with client...
  client.run_model(model_id, input)
  # implicitly close the connection when exiting the scope

When possible we should encourage people to use the interface above, but with statement resources management does not fit all use cases (for instance in our notebooks we need to keep the connection opened between cells). In these circumstances, the connection should be closed manually via the .close() method as shown just below (this is similar to what is currently done) :

client = blindai.client.connect(addr="localhost", simulation=True)
# do something with client...
client.run_model(model_id, input)
# explicitly close the connection
client.close()

Related Issue

Closes #40

Type of change

This PR introduces a breaking change.

How Has This Been Tested?

The tests have been updated to use the new interface when appropriate.

Checklist:

github-actions[bot] commented 2 years ago

☂️ Python Cov

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
374 329 88% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
client/blindai/client.py 90% 🟢
TOTAL 90% 🟢

updated for commit: 84ef7a2 by action🐍

cchudant commented 2 years ago

This is cool! Using a decorator like that is cool.

What is contextlib? is this a new pypi requirement or is it baked in python by default?

This is not backward compatible if i am guessing right. We need to update the blog posts when we will make a release.

clauverjat commented 2 years ago

What is contextlib? is this a new pypi requirement or is it baked in python by default

contextlib is not a new pypi requirement, it is part of the Python standard library. It was introduced in Python 3.5, but we need at least Python 3.6 because we use the AbstractContextManager. As explained in https://docs.python.org/3/library/contextlib.html, the module provides utilities for common tasks involving the with statement.

This is not backward compatible if i am guessing right. We need to update the blog posts when we will make a release.

Indeed the change is not backward compatible. As stated above "this PR introduces a breaking change". But I think we will need to update the blog posts either way because #73 adds a mandatory model_id argument to the client.run_model() method.