Skip to content
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.

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

FluentConnection class refactoring #2609

Closed
seanpearsonuk opened this issue Mar 25, 2024 · 1 comment · Fixed by #2617
Closed

FluentConnection class refactoring #2609

seanpearsonuk opened this issue Mar 25, 2024 · 1 comment · Fixed by #2617
Assignees

Comments

@seanpearsonuk
Copy link
Collaborator

seanpearsonuk commented Mar 25, 2024

FluentConnectionProperties encapsulates the business data for FluentConnection. FluentConnection should not have other dependencies.

Glancing at its constructor, launcher_args stands out as a false dependency of FluentConnection.
The constructor uses it to obtain any SLURM job ID: self._slurm_job_id = launcher_args and launcher_args.get("slurm_job_id"). An optional slurm_job_id is a better (imperfect) construction dependency. Note that self._slurm_job_id is used only at exit. This implies that we can later layer things to keep such details out of the raw connection class.

FluentConnection::__init__ creates a SchemeEvalService. This is too concrete. What is it used for?

            fluent_host_pid = self.scheme_eval.scheme_eval("(cx-client-id)")
            cortex_host = self.scheme_eval.scheme_eval("(cx-cortex-host)")
            cortex_pid = self.scheme_eval.scheme_eval("(cx-cortex-id)")
            cortex_pwd = self.scheme_eval.scheme_eval("(cortex-pwd)")
        build_time = self.scheme_eval.scheme_eval("(inquire-build-time)")
        build_id = self.scheme_eval.scheme_eval("(inquire-build-id)")
        rev = self.scheme_eval.scheme_eval("(inquire-src-vcs-id)")
        branch = self.scheme_eval.scheme_eval("(inquire-src-vcs-branch)")
        scheme_eval.exec(("(exit-server)",))

We should design Python interfaces for these. In the first instance, we can implement them in Python, wrapping SchemeEval. Later, we can move the implementation into the server behind gRPC. The first two chunks can all go behind a ServerInfo interface. The last can be in a ServerControl interface.

@seanpearsonuk
Copy link
Collaborator Author

@prmukherj Noting that I reopened this in April but since no reason was given, I will assume that there is nothing more to do until we see something concrete in future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 2021-2024
Development

Successfully merging a pull request may close this issue.

2 participants