Skip to content

RSDK-13355: remove reconfigure#1167

Open
aldenh-viam wants to merge 3 commits into
viamrobotics:mainfrom
aldenh-viam:push-tllqywwkkpkx
Open

RSDK-13355: remove reconfigure#1167
aldenh-viam wants to merge 3 commits into
viamrobotics:mainfrom
aldenh-viam:push-tllqywwkkpkx

Conversation

@aldenh-viam
Copy link
Copy Markdown
Contributor

Modules no longer need to implement Reconfigure after viamrobotics/rdk#5811, since ReconfigureResource will not be called by viam-server anymore and it will instead always close (RemoveResource) and recreate (AddResource) the resource.

@aldenh-viam aldenh-viam force-pushed the push-tllqywwkkpkx branch 3 times, most recently from 9a2f335 to 7a26220 Compare March 17, 2026 19:03
Comment thread src/viam/module/types.py
from viam.proto.app.robot import ComponentConfig
from viam.proto.common import ResourceName
from viam.resource.base import ResourceBase


@runtime_checkable
class Reconfigurable(Protocol):
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we remove this class completely? Or not, because it would crash at runtime for users who bump the SDK version but forget to remove it from their code?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep it for now, but can you add a deprecated warning to it please?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean use __init_subclass__ or something so it prints at runtime when something subclasses this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah exactly. Not sure if __init_subclass__ works for protocols. If not, maybe we do just remove the protocl. I think it's been removed in the other SDKs already, correct?

Copy link
Copy Markdown
Contributor Author

@aldenh-viam aldenh-viam May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__init_subclass__ works for me:

$ python3 -W always  # always show warnings
Python 3.13.12 (main, Feb  3 2026, 17:53:27) [GCC 15.2.0] on linux
>>> from viam.module.types import Reconfigurable
>>> class Test(Reconfigurable):
...     pass
...
<frozen abc>:106: DeprecationWarning: Reconfigure is deprecated, and resources will always rebuild. It is not necessary to implement Reconfigurable.
$ git diff src/viam/module/types.py
diff --git a/src/viam/module/types.py b/src/viam/module/types.py
index 4930653426..3ac0e77067 100644
--- a/src/viam/module/types.py
+++ b/src/viam/module/types.py
@@ -1,9 +1,12 @@
 from typing import Any, Mapping, Optional, Protocol, runtime_checkable
+import warnings


 @runtime_checkable
 class Reconfigurable(Protocol):
     """The Reconfigurable protocol defines the requirements for making a resource Reconfigurable"""
+    def __init_subclass__(*args, **kwargs):
+        warnings.warn("Reconfigure is deprecated, and resources will always rebuild. It is not necessary to implement Reconfigurable.", DeprecationWarning, stacklevel=2)

I think it's been removed in the other SDKs already, correct?

Yes, it's completely removed in C++ already, and will be in Go once the main PR gets merged.

@aldenh-viam aldenh-viam marked this pull request as ready for review March 18, 2026 13:35
@aldenh-viam aldenh-viam requested a review from a team as a code owner March 18, 2026 13:35
Copy link
Copy Markdown
Member

@njooma njooma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 2 things:

  1. Add the deprecated warning to Reconfigurable protocol
  2. Can you do a search over the python repo for reconfigure? There are still a few places in (particularly docs) where we have outdated information

Comment thread src/viam/module/types.py
from viam.proto.app.robot import ComponentConfig
from viam.proto.common import ResourceName
from viam.resource.base import ResourceBase


@runtime_checkable
class Reconfigurable(Protocol):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep it for now, but can you add a deprecated warning to it please?

Copy link
Copy Markdown
Member

@stuqdog stuqdog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, modulo Naveed's comments.

@njooma
Copy link
Copy Markdown
Member

njooma commented Apr 6, 2026

@aldenh-viam checking in on status -- trying to avoid becoming stale

@aldenh-viam
Copy link
Copy Markdown
Contributor Author

@aldenh-viam checking in on status -- trying to avoid becoming stale

LMK about my question in the above thread. Are you looking for a deprecation warning to print at runtime or just a comment? I searched the repo, but can't tell if there's an existing style I should follow.

@aldenh-viam aldenh-viam requested a review from njooma May 20, 2026 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants