refactor: timeout refactor to use CLI option value

[etiennedm]

This is to address my comment in #71. Do note that I have not updated the smpclient version in the dependencies. I'm not sure how to do it since I'm guessing it will need a release of smpclient first.

See intercreate/smpclient#94 for the necessary changes in smpclient.

[etiennedm]

I'm assuming the linting fails because it's not using the updated version of smpclient. It works locally. Any trick here to make it work/test it is ok before bumping the version? I tried to specify the dependency as smpclient = {git = "https://github.com/etiennedm/smpclient.git", branch = "smpclient-timeout-refactor" } but poetry is not happy with it (something to do with dynamic versioning and strict mode I'm guessing?)

[JPHutchins]

I'm assuming the linting fails because it's not using the updated version of smpclient. It works locally. Any trick here to make it work/test it is ok before bumping the version? I tried to specify the dependency as smpclient = {git = "https://github.com/etiennedm/smpclient.git", branch = "smpclient-timeout-refactor" } but poetry is not happy with it (something to do with dynamic versioning and strict mode I'm guessing?)

Running poetry lock and pushing the changes should work. But the we have to remember to retarget pypi after the upstream is released, which won't be too long.

[etiennedm]

I'm not sure what needs to be done, I'm not very familiar with poetry. Right now my poetry.lock diff is showing a bunch of changes, and I have smpclient = { path = "../smpclient", develop = true } for the smpclient dependency.

I guess the easiest would be to do a release of smpclient once you merge intercreate/smpclient#94, which we could then target in the dependencies of smpmgr?

[JPHutchins]

I'm not sure what needs to be done, I'm not very familiar with poetry. Right now my poetry.lock diff is showing a bunch of changes, and I have smpclient = { path = "../smpclient", develop = true } for the smpclient dependency.

I guess the easiest would be to do a release of smpclient once you merge intercreate/smpclient#94, which we could then target in the dependencies of smpmgr?

Yeah, that's easiest in the sense that it doesn't require coming back to this PR to update the pyproject and lock.

[JPHutchins]

@etiennedm If you don't mind, I'll rebase this and get the feature merged in a new PR.

[etiennedm]

Sure, go for it!

[JPHutchins]

Am I right in thinking that the single timeout arg applies to connection, and then to each subsequent command? Trying to remember what we decided. If that's right, I want to confirm that I'm OK with it as a stop gap until it's refactored to separate the initial connection timeout from subsequent request timeouts.

[Copilot]

Pull request overview

This PR refactors timeout handling so the CLI-provided --timeout value is applied via the SMPClient instance (rather than being threaded through connect_with_spinner / smp_request call sites), aligning with the timeout unification request raised in #71.

Changes:

• Pass options.timeout into SMPClient construction and remove per-call timeout wiring for connect_with_spinner(...).
• Simplify smp_request(...) call sites by removing the options parameter and relying on client defaults / per-command overrides.
• Bump smpclient dependency to ^6.1.0 (and refresh poetry.lock).

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
smpmgr/common.py	Centralizes timeout behavior in client creation; updates helper APIs (connect_with_spinner, smp_request).
smpmgr/main.py	Updates upgrade flow to new helper signatures; continues to support request-specific timeouts via smp_request.
smpmgr/shell_management.py	Makes shell command timeout optional and forwards it to smp_request.
smpmgr/stat_management.py	Updates stats commands to new connect_with_spinner / smp_request signatures.
smpmgr/os_management.py	Updates OS management commands to new helper signatures.
smpmgr/image_management.py	Updates image management commands to new helper signatures.
smpmgr/file_management.py	Updates file management commands to new helper signatures.
smpmgr/enumeration_management.py	Updates enumeration commands to new helper signatures.
smpmgr/user/intercreate.py	Updates Intercreate upload to new connect_with_spinner signature.
plugins/example_group.py	Updates plugin example to new connect_with_spinner signature.
plugins/another/another_group.py	Updates plugin example to new connect_with_spinner signature.
pyproject.toml	Bumps smpclient dependency version.
poetry.lock	Updates lockfile to match dependency resolution (including smpclient 6.1.0).

Comments suppressed due to low confidence (1)

smpmgr/common.py:128

• smp_request always passes a second argument to smpclient.request(...) even when timeout_s is None. Since other code in this repo calls smpclient.request(request) without providing a timeout, passing None here can change semantics (e.g., disable the client's default timeout or raise in downstream code). Consider calling smpclient.request(request) when timeout_s is None, and only passing the timeout argument when it’s a real float value.

        description = description or f"Waiting for response to {request.__class__.__name__}..."
        task = progress.add_task(description=description, total=None)
        try:
            r = await smpclient.request(request, timeout_s)
            progress.update(task, description=f"{description} OK", completed=True)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[etiennedm]

Yes the single CLI timeout arg applies to the connection and subsequent commands when it is None, if I understood your question correctly.