global _api_client

[PearsonWhite]

Use global api_client instead of passing it around

[AlbertoSoutullo]

Hmm I dontt really agree with this approach.

First, there are a lot of places where api_client is still being used, which makes the PR a bit weird to review.
Then, if we want to simplify the usage of api_clientI would not go with global.
Create something like a session:

@dataclass
class KubeSession:
    kube_config: str
    api_client: client.ApiClient

    @classmethod
    def from_config(cls, kube_config: str) -> "KubeSession":
        config.load_kube_config(config_file=kube_config)
        return cls(kube_config=kube_config, api_client=client.ApiClient())

    def close(self) -> None:
        self.api_client.close()

Then you can do something like:

session = KubeSession.from_config(str(kube_config))
try:
    await experiment.run(session, args, values_yaml)
finally:
    session.close()

This allows you to pass session through experiment boundaries only:

BaseExperiment.run(session, args, values_yaml)
BaseExperiment._run(session, ...)
RegressionNodes.run(session, ...)

Then inside BaseExperiment.run, you can store self._api_client = session.api_client for convenience, and remove the global entirely.
There are probably even better ways to do this, but definitely this global approach... I don't see the benefit of it.

[PearsonWhite]

Hmm I dontt really agree with this approach.

We have to load the kube_config once at the beginning, and then it's globally initialized in the library, even if we didn't keep it here in the the kube_utils.py. It's just the paradigm of the library.

config.load_kube_config(config_file=_kube_config)

Then, if we want to simplify the usage of api_clientI would not go with global. Create something like a session:

As for the wrapper, what do you think about wrapping the kube_utils in a class, storing the api_client there, and injecting that into the experiment?

class KubeUtils(BaseModel):
    kube_config : str
    api_client : client.ApiClient

    @staticmethod
    def init_with_config(kube_config : str) -> "KubeUtils":
        config.load_kube_config(config_file=kube_config)
        api_client = client.ApiClient()
        return KubeUtils(kube_config=kube_config, api_client=api_client)

    def get_pods_for_statefulset(self, name: str, namespace: str) -> Iterable[V1Pod]:
        apps_api = self.client.AppsV1Api(_api_client)
        core_api = self.client.CoreV1Api(_api_client)

        kwargs = {
            "namespace": namespace,
        }
        ...

class BaseExperiment(ABC, BaseModel):
    kube_utils : KubeUtils

    async def deploy(self, ...):
        ...
        if isinstance(deployment, V1Deployable):
            deployment = self.kube_utils.k8s_obj_to_dict(deployment)
        ...

Otherwise, I'm happy to just close this for now. I already have some local changes for moving all of the run fields, including the api_client, into the Experiment classes as per this comment:

10ksim/src/deployments/experiments/base_experiment.py

Line 124 in d03c8a2

# TODO: store api_client, stack, and (experiment) args in self and remove as function params.

[AlbertoSoutullo]

As for the wrapper, what do you think about wrapping the kube_utils in a class, storing the api_client there, and injecting that into the experiment?

I think this sounds good. But we already have a KubernetesManager. It would be better to merge this to maintain Kubernetes operations encapsulated in its own package. But definitely not globaling the api_client.