Internal deploy python client functionality - #218
Conversation
| def __eq__(self, other): | ||
| return self.connection == other.connection | ||
|
|
||
| def register_upload_bundle_fn( |
There was a problem hiding this comment.
Not sure if this is the best way of allowing users to use their own backend for storing model bundles (in internal mode). Think this is better than hardcoding any stuff with smart_open/boto3, since I don't want to introduce another dependency.
| """ | ||
| self.connection = Connection(api_key, endpoint) | ||
| self.is_internal = is_internal | ||
| self.upload_bundle_fn: Optional[Callable[[str, str], None]] = None |
There was a problem hiding this comment.
It gets set elsewhere, but I think people (and my IDE) say it's good to initialize all instance variables inside init
| ) | ||
| if self.is_internal: | ||
| payload.update( | ||
| aws_role=aws_role, results_s3_bucket=results_s3_bucket |
There was a problem hiding this comment.
should check aws_role and results_s3_bucket are not None
Tianwei-She
left a comment
Tianwei-She
left a comment
There was a problem hiding this comment.
I'm not sure if this is the best design, but I think it's ok to merge for now
| def __init__( | ||
| self, | ||
| api_key: str, | ||
| endpoint: str = SCALE_DEPLOY_ENDPOINT, |
There was a problem hiding this comment.
hmm should we have this as a default? won't this not work if it's wrong
There was a problem hiding this comment.
I think in the long term we'd want it to be a default, i.e. have SCALE_DEPLOY_ENDPOINT equal to api.scale.com/v1/deploy or whatever it turns out to be. Think users would be more wrong specifying endpoint than us.
| self, | ||
| api_key: str, | ||
| endpoint: str = SCALE_DEPLOY_ENDPOINT, | ||
| is_internal: bool = False, |
There was a problem hiding this comment.
hmm didn't we say we weren't gonna have an internal / external split so we don't commit this to a public repo?
ok with it i guess
There was a problem hiding this comment.
I think having this flag by itself isn't the end of the world. The critical thing is to ensure that we're not leaking secrets. Less critical but still important is to ensure we're not leaking too many implementation details, which I think other parts of this PR are.
| self, | ||
| api_key: str, | ||
| endpoint: str = SCALE_DEPLOY_ENDPOINT, | ||
| is_internal: bool = False, |
There was a problem hiding this comment.
I think having this flag by itself isn't the end of the world. The critical thing is to ensure that we're not leaking secrets. Less critical but still important is to ensure we're not leaking too many implementation details, which I think other parts of this PR are.
4 participants
The client operates in two modes, external mode (default) and self-hosted mode.
Also add a few extra functions for the missing API calls (e.g. delete bundle/endpoint, put endpoint)
For uploading model bundles in self-hosted mode, there is an
upload_bundle_fnthat needs to be passed to the client. This allows clients to specify their own model bundle storage backends. An example implementation isn.b. the code assumes that the REST APIs for the public route and the internal route are nearly identical, e.g. creating a model bundle is always at
POST /model_bundle/for example, and it takes in a JSON with a keyurl.Testing:
Ran a testing script that
using the client in internal and external mode. Calls all succeed, the task doesn't return a result because of some other bug though.