RSDK-13355: remove reconfigure#1167
Conversation
aldenh-viam
force-pushed
the
push-tllqywwkkpkx
branch
2 times, most recently
from
029f942 to
9a2f335
Compare
aldenh-viam
force-pushed
the
push-tllqywwkkpkx
branch
from
9a2f335 to
7a26220
Compare
| from viam.proto.app.robot import ComponentConfig | ||
| from viam.proto.common import ResourceName | ||
| from viam.resource.base import ResourceBase | ||
|
|
||
|
|
||
| @runtime_checkable | ||
| class Reconfigurable(Protocol): |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Let's keep it for now, but can you add a deprecated warning to it please?
There was a problem hiding this comment.
You mean use __init_subclass__ or something so it prints at runtime when something subclasses this?
njooma
left a comment
njooma
left a comment
There was a problem hiding this comment.
Looks good! 2 things:
- Add the deprecated warning to
Reconfigurableprotocol - 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
| from viam.proto.app.robot import ComponentConfig | ||
| from viam.proto.common import ResourceName | ||
| from viam.resource.base import ResourceBase | ||
|
|
||
|
|
||
| @runtime_checkable | ||
| class Reconfigurable(Protocol): |
There was a problem hiding this comment.
Let's keep it for now, but can you add a deprecated warning to it please?
stuqdog
left a comment
stuqdog
left a comment
There was a problem hiding this comment.
lgtm, modulo Naveed's comments.
|
@aldenh-viam checking in on status -- trying to avoid becoming stale |
3 participants
Modules no longer need to implement
Reconfigureafter viamrobotics/rdk#5811, sinceReconfigureResourcewill not be called byviam-serveranymore and it will instead always close (RemoveResource) and recreate (AddResource) the resource.