canonical / operator

Pure Python framework for writing Juju charms
Apache License 2.0
246 stars 119 forks source link

Harness: raise when charm code exceeds juju's relation data limits #1304

Closed sed-i closed 1 month ago

sed-i commented 3 months ago

Without compressing reldata, cos-proxy is approaching the value length limit for relation data. It would be nice if harness/scenario flagged when the code under test attempts to exceed key/value length limit.

dimaqq commented 3 months ago

Off the top of my head, I'm not clear if relation data is set in its entirety, or could be set one key at a time. In the latter case, ops may not know the size of other keys. Let's check this.

dimaqq commented 3 months ago

The posted limits are the key length (256) and value size for each key (64K). There doesn't seems to be a limit on the total databag size, is there?

Note that mongo somehing-something limit is 16MB, which means that fully loaded databag update is guaranteed to break things (at least juju status / all watcher stream). This is probably a hard limit until Juju 4 / dqlite.

tonyandrewmeyer commented 3 months ago

I don't think we should add this to Harness, but I do think it would be worth adding to Scenario, assuming we can get clear numbers on what the limits are.

Let's add this to our next chat with the Juju team.

tonyandrewmeyer commented 2 months ago

Without compressing reldata, cos-proxy is approaching the value length limit for relation data. It would be nice if harness/scenario flagged when the code under test attempts to exceed key/value length limit.

We talked about this with the Juju team, and the charm state limits linked there are for the charm state itself, not the relation data (for when use_juju_for_storage is True). Given that we want people to not store charm state in Juju, we don't think it's worth adding warnings for that case.

For relations, the only limit at the moment is from Mongo. We feel like it would be difficult to get an accurate failure/warning in the test frameworks in that case. Ian's going to talk to the Juju team about adding limits in Juju 4 (instead of just moving to being limited by dqlite), and if that happens then we'd definitely be in favour of also adding those to the test framework.

benhoyt commented 1 month ago

Given

We feel like it would be difficult to get an accurate failure/warning in the test frameworks in that case.

We're going to close this.

benhoyt commented 1 month ago

If Juju adds defined limits in Juju 4, we can reconsider.