GSK-2896 Automatically generate a Kernel based on dependencies #1829
Conversation
| if len(matching_kernels) > 0: | ||
| return matching_kernels[0]["name"] | ||
|
|
||
| self._session.post( |
There was a problem hiding this comment.
Should we check the result here? Cuz for "PROCESS" kernel, backend will create an env as well. This can fail sometimes
There was a problem hiding this comment.
I think this object is doing it automatically, contrary to request ?
There was a problem hiding this comment.
Maybe we should change backend, to make start install deps and freeze if needed ?
We could make it so it save in DB the kernel, and then immediatly launch an async task in job manager to do install + freeze (+start ?) if needed ?
There was a problem hiding this comment.
I think this object is doing it automatically, contrary to request ?
Yes, you are right. So it will raise the exception in case of failed creation, right?
giskard/client/giskard_client.py
Outdated
| def initialize_kernel(self): | ||
| python_version = f"{sys.version_info[0]}.{sys.version_info[1]}" | ||
| base_kernel_name = f"{gethostname()}-{python_version}" | ||
| kernel_name = f"{base_kernel_name}-{uuid.uuid4()}" |
There was a problem hiding this comment.
I think we could go with something simpler, like <project_key>_<auto>
| @@ -209,6 +245,10 @@ def create_project(self, project_key: str, name: str, kernel_name: str, descript | |||
| "name": anonymize(name), | |||
| }, | |||
| ) | |||
|
|
|||
| if kernel_name is None: | |||
There was a problem hiding this comment.
Only issue here for me is if you try to create a project already existing, you will re-create a kernel everytime.
That's why changing kernel_name to <project_key>_<auto> (or at least without random/uuid in name) will protect that, since kernel creation is checking that
There was a problem hiding this comment.
I updated the kernel creation to use the project_key. It should not create a new kernel if the project key already exists (granted it has same dependencies)
| if len(matching_kernels) > 0: | ||
| return matching_kernels[0]["name"] | ||
|
|
||
| self._session.post( |
There was a problem hiding this comment.
I think this object is doing it automatically, contrary to request ?
| # A different kernel exists that uses the same name | ||
| kernel_name = f"{kernel_name}_{uuid.uuid4()}" | ||
|
|
||
| self._session.post( |
There was a problem hiding this comment.
Can we extract this into a method like create_kernel in Giskard client?
I am also trying to do something similar for list kernels. Otherwise, I can do it on my side after merging this branch.
It's done, when the key already exists it'll reuse kernel of same key and won't create a new one. |
|
|
Looks good! Just need to test it on my side |
Description
Automatically generate a Kernel based on dependencies.
If any kernel match current dependencies, then use it instead
Related Issue
Type of Change