defer() pattern tasks

[tonyandrewmeyer]

Follow-up actions from the defer analysis:

• Investigate removing the defer() method from event classes that can’t defer (changing to AttributeError from RuntimeError, so that IDEs and linters easily detect it)
• Remove the ability to defer (currently only Action, SecretRemoved, SecretExpired) from other relevant classes (CollectStatus, maybe some of the teardown events)
• Discourage “if container.can_connect()” guards
  • Reword the API reference docs so that it doesn’t push people this way
  • Check other docs (e.g. the tutorial) to see if improvements can be made
  • Maybe some additional reach-out
• For “I need this collection of things to be ready”: where possible, encourage a common code path and return without deferring (trusting that the last event will take care of everything).
  • Update docs
    • "Deferring Events: Details and Dilemmas" (Discourse) - there are some other references to defer() on juju.is.docs/sdk, but they are all explaining what defer() does, and already have appropriate wording around taking care with its use, and don't seem to need anything additional
    • SDK - this has a good section on taking care when deciding if defer() should be used, and two examples of where it's not a good choice. This is already one of the longest method docstrings we have - I don't think it's the right place to go into more detail (at most we could add a link to more documentation)
  • Review key charms from each team and reach out where applicable with suggestions
  • Maybe a Discourse post encouraging best practices
• Feature request for a Juju retry

[tonyandrewmeyer]

• Investigate removing the defer() method from event classes that can’t defer (changing to AttributeError from RuntimeError, so that IDEs and linters easily detect it)
• Remove the ability to defer (currently only Action, SecretRemoved, SecretExpired) from other relevant classes (CollectStatus, maybe some of the teardown events)

Both in #1122 (TBD if we will remove or not).

• Discourage “if container.can_connect()” guards

  • Reword the API reference docs so that it doesn’t push people this way

#1123

[tonyandrewmeyer]

• Check other docs (e.g. the tutorial) to see if improvements can be made

Tutorial:

• https://discourse.charmhub.io/t/make-your-charm-configurable/7401
• https://discourse.charmhub.io/t/expose-the-version-of-the-application-behind-your-charm/7406
• https://discourse.charmhub.io/t/integrate-your-charm-with-postgresql/7414

Other:

• https://discourse.charmhub.io/t/storedstate-uses-limitations/5740
• https://discourse.charmhub.io/t/event-container-pebble-ready/6468

That seems to be all the cases on Discourse, so hopefully that (plus the already done API docs) should be everywhere.

[tonyandrewmeyer]

• "Deferring Events: Details and Dilemmas" (Discourse) - there are some other references to defer() on juju.is.docs/sdk, but they are all explaining what defer() does, and already have appropriate wording around taking care with its use, and don't seem to need anything additional

@benhoyt could you please take a look at these suggested changes to that doc? Happy to schedule a call to go over them together if that works best.

[benhoyt]

@tonyandrewmeyer Added a couple of comments. I think there's more work to be done on that doc, but I think the improvements you've made are all in the right direction -- thanks!

[tonyandrewmeyer]

• Review key charms from each team and reach out where applicable with suggestions

I've reviewed all the use of defer() in the "key charms" (summaries are in the charm spreadsheet), and there's a lot of can_connect() + defer() that I would suggest is better handled by catching ConnectionError but would still be defer(). Several charms already use the holistic pattern, and where the others aren't, there are cases where (given the unpredictability of ordering) defer() is still required.

In short: I didn't find any charm where there are significant areas for improvement in defer() use - outside of a first-class Juju reemit method being added.

[tonyandrewmeyer]

• Maybe a Discourse post encouraging best practices

@benhoyt would you be able to find some time this week to review this guidance doc? Thanks!