Robot server maximum quick transfer protocols

[vegano1]

Overview

Quick Transfer protocols are meant to be easy to set up and use protocols from the ODD, so

1. We don't want to crowd the screen real estate with unlimited protocols
2. We want to encourage users to delete and create new quick transfer protocols as they see fit
3. We want to limit the number of protocols stored on the robot so performance is not impacted.

Let's introduce a robot server setting maximum_quick_transfer_protocols to set the maximum number of quick transfer protocols. Then let's use this setting to return a 400 error if we have reached the limit,

Closes: PLAT-314 PLAT-317

Test Plan

• Make sure we store quick transfer protocols until we hit the limit
• Once the protocol limit is hit, make sure we don't store the new protocol and respond with a 400 error

Changelog

• Add maximum_quick_transfer_protocols robot server config
• Limit the number of quick transfer protocols based on the above config and return a 400 error once the limit is reached

Review requests

• Makes sense?

Risk assessment

Low, unreleased

[sfoster1]

Is the intent here that it's up to the client to react to this and delete a protocol record? why wouldn't we have automatic storage management where the oldest protocol is deleted when you upload a new one?

[vegano1]

Is the intent here that it's up to the client to react to this and delete a protocol record? why wouldn't we have automatic storage management where the oldest protocol is deleted when you upload a new one?

Yeah, the idea is that the client is responsible for managing the number of quick transfer protocols.
Also, Quick transfer protocols don't have a run history, since the client deletes the run once finished.
This means the quick transfer protocols will be marked as "unused" and likely be deleted by the existing mechanism.

[CaseyBatten]

Looks good overall, I think these additions make sense and are in line with how we've done similar cases. Note below about a minor nitpick on how we gate the maximum number of protocols, but mostly a semantics thing. Aside from that all looks good, but I would update the title of the PR to be in line with our convention before merging.

[CaseyBatten]

This check makes sense but would it be more semantically accurate to test for something like this? :

if (len(quick_transfer_protocols) + 1) > maximum_quick_transfer_protocols:

Because the protocol store is the selection of currently saved protocols, and we can assume it will never have a value greater than our maximum because of this creation case. Checking >= seems to assume a failure of that rule, where as the len(...) + 1 greater than case assumes that we are below or at the limit and are checking to see if we would exceed it by adding this new protocol.

Either way they will achieve the same result, completely up to you.

[sfoster1]

Looks good! It's a bit early for that comment as noted, but I don't think it's worth a big fuss.

[sfoster1]

I'm not sure what the right way to do this sort of thing is, but it is the case that this comment is inaccurate for the state of the code as-of merging this protocol - it's only in #15591 that the autodeletion is actually introduced. I think we should merge it; if we were stricter, I'd say we should have it not mention autodeletion here and then change it in that upcoming pr, but whatever.