launchpad-reviewers team mailing list archive

[Colin Watson <mp+439082@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad-layers:endpoint-from-flag into launchpad-layers:main.

Commit message:
Use endpoint_from_flag rather than handler arguments

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-layers/+git/launchpad-layers/+merge/439082

https://charmsreactive.readthedocs.io/en/latest/charms.reactive.decorators.html
says:

  For backwards compatibility, some decorators will pass endpoint
  instances if the handler function specifies them as arguments.
  However, explicit instance access using `endpoint_from_flag` is
  recommended, because ensuring proper argument order can be confusing:
  they are passed in bottom-up, left-to-right, and no negative or
  ambiguous decorators, such as `when_not()` or `when_any()` will ever
  pass arguments.

I also ran into some weird behaviour during an `upgrade-charm` attempt where the backward-compatibility mode doesn't quite seem to work, so use the recommended explicit form instead.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad-layers:endpoint-from-flag into launchpad-layers:main.

diff --git a/launchpad-base/reactive/launchpad-base.py b/launchpad-base/reactive/launchpad-base.py
index 9145b65..b3bd439 100644
--- a/launchpad-base/reactive/launchpad-base.py
+++ b/launchpad-base/reactive/launchpad-base.py
@@ -12,7 +12,14 @@ from charms.launchpad.base import (
     strip_dsn_authentication,
     update_pgpass,
 )
-from charms.reactive import hook, remove_state, set_state, when, when_not
+from charms.reactive import (
+    endpoint_from_flag,
+    hook,
+    remove_state,
+    set_state,
+    when,
+    when_not,
+)
 from ols import base, postgres
 from psycopg2.extensions import parse_dsn
 
@@ -30,7 +37,8 @@ base.create_virtualenv = create_virtualenv
 
 
 @when("rabbitmq.connected")
-def prepare_rabbitmq(rabbitmq):
+def prepare_rabbitmq():
+    rabbitmq = endpoint_from_flag("rabbitmq.connected")
     config = hookenv.config()
     rabbitmq.request_access(config["rabbitmq_user"], config["domain"])
 
@@ -50,7 +58,9 @@ def get_rabbitmq_uris(rabbitmq):
 
 @when("ols.configured", "db.master.available", "rabbitmq.available")
 @when_not("launchpad.base.configured")
-def configure(db, rabbitmq):
+def configure():
+    db = endpoint_from_flag("db.master.available")
+    rabbitmq = endpoint_from_flag("rabbitmq.available")
     ensure_lp_directories()
     config = get_service_config()
     db_primary, db_standby = postgres.get_db_uris(db)
@@ -109,6 +119,6 @@ def config_changed():
 
 
 @hook("{requires:rabbitmq}-relation-changed")
-def rabbitmq_relation_changed(rabbitmq):
+def rabbitmq_relation_changed():
     remove_state("launchpad.base.configured")
     remove_state("service.configured")