Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Higher level primitives for databag handling? #1501

Open
sed-i opened this issue Dec 15, 2024 · 1 comment
Open

Higher level primitives for databag handling? #1501

sed-i opened this issue Dec 15, 2024 · 1 comment
Labels
feature New feature or request needs design needs more thought or spec

Comments

@sed-i
Copy link
Contributor

sed-i commented Dec 15, 2024

Unless we set limit: 1, charms should expect handling multiple relations. As a result, we always iterate over all relations' data to push/pull data.

I wonder if adding some primitives to ops would help with pitfalls and improve code quality. Because it's more straightforward to reason about a single databag.

Update remote unit data

If ops had a function,

def push_unit_data(self, rel_name, updater):
	for relation in self.model.relations.get(rel_name, []):
		updater(relation.data[self.charm.unit])

Then instead of:

for relation in self.charm.model.relations.get(self.name, []):
	relation.data[self.charm.unit].update(self._generate_relation_data())

we could have:

push_unit_data(self.name, updater)

def updater(self, databag):
	databag.update(self._generate_relation_data())

Update remote app data

If ops had a function,

def push_app_data(self, rel_name, updater):
	for relation in self.model.relations.get(rel_name, []):
		updater(relation.data[self.charm.app])

then instead of

for relation in self.model.relations[self._relation_name]:
	if not self._charm.unit.is_leader():
		return

	if alert_rules_as_dict := self.alert_rules.as_dict():
		relation.data[self._charm.app]["alert_rules"] = json.dumps(alert_rules_as_dict)

we could have:

push_app_data(self._relation_name, updater)

def updater(self, databag):
	if not self.charm.unit.is_leader():
		return

	if alert_rules_as_dict := self.alert_rules.as_dict():
		databag = json.dumps(alert_rules_as_dict)

Fetch remote unit data

If ops had a function,

def pull_remote_unit_data(self, rel_name, getter) -> Dict[Unit, Any]:
    ret = {}
    for relation in self.model.relations.get(self._relation_name, []):
        for unit in relation.units:
            if unit.app is self._charm.app:
                # This is a peer unit
                continue
            ret[unit] = getter(relation.data[unit])
    return ret

then instead of

endpoints = []
for relation in self.model.relations[self._relation_name]:
	for unit in relation.units:
		if unit.app is self._charm.app:
			# This is a peer unit
			continue

		remote_write = relation.data[unit].get("remote_write")
		if remote_write:
			deserialized_remote_write = json.loads(remote_write)
			endpoints.append(
				{
					"url": deserialized_remote_write["url"],
				}
			)

we could have:

endpoints = pull_remote_unit_data(self._relation_name, getter).values()

def getter(databag):
	remote_write = databag.get("remote_write")
	if remote_write:
		deserialized_remote_write = json.loads(remote_write)
		return {"url": deserialized_remote_write["url"]})
@tonyandrewmeyer tonyandrewmeyer added feature New feature or request needs design needs more thought or spec labels Dec 17, 2024
@tonyandrewmeyer
Copy link
Contributor

Interesting suggestion, thanks @sed-i !

I think in general we're reluctant to add multiple ways of doing things, but I can see how some of the code is more readable.

For now:

  • @james-garner-canonical is spending a lot of the current cycle looking at what libs Charm-Tech can create/maintain to make charming better. We've talked about some of that potentially being experimentation with new patterns that might then later make it into ops, so this could be a candidate for that.
  • I'm working on a spec (currently working on getting it from braindump to draft) to support typing and validating relation data. We're potentially going to need a new interface for that anyway, so while I'm working on it, I'll see if an approach like this would also fit in for that.

When the next planning period is coming around, we'll look through all the tickets, including this one, and see what's progressed and what we might want to work on for 25.10.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request needs design needs more thought or spec
Projects
None yet
Development

No branches or pull requests

2 participants