launchpad-reviewers team mailing list archive

[Colin Watson <mp+448544@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:charm-assets-extra-symlinks into launchpad:master.

Commit message:
charm: Add "lp" and "yui" convoy symlinks in devmode

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

In "devmode" (set for local development deployments, but not otherwise), the application server produces links such as `/+combo/?yui/yui/yui-min.js&lp/meta.js&yui/loader/loader-min.js` rather than `/+combo/rev<commit-id>/?yui/yui/yui-min.js&lp/meta.js&yui/loader/loader-min.js`.

This is due to an assumption that revision-specific JavaScript bundles probably won't be available in typical devmode setups (or indeed needed, since they exist mainly for high availability of the appropriate bundle revisions during appserver upgrades), which is less true now that we have that system packaged up in the assets charm; but it's still true for most non-charmed setups using `make run`, and it would take some effort to detach this particular behaviour from the various other adjustments that devmode makes, so I'd rather not change that link generation code right now.

However, it's helpful for those non-revision-specific JavaScript bundles to work in charmed devmode setups, since (once we have frontend charms finished) that allows deploying a Launchpad stack using the Mojo spec and having working JavaScript in the resulting web UI.  A small tweak to the assets charm is enough to make that work: in devmode, make `/srv/launchpad-assets/convoy/{lp,yui}` be symlinks to the appropriate subdirectories of the current payload revision.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:charm-assets-extra-symlinks into launchpad:master.

diff --git a/charm/launchpad-assets/config.yaml b/charm/launchpad-assets/config.yaml
index c440578..c3b3ce8 100644
--- a/charm/launchpad-assets/config.yaml
+++ b/charm/launchpad-assets/config.yaml
@@ -1,4 +1,8 @@
 options:
+  devmode:
+    type: boolean
+    description: Is this server running in dev mode?
+    default: true
   domain:
     type: string
     description: Domain name for this instance.
diff --git a/charm/launchpad-assets/reactive/launchpad-assets.py b/charm/launchpad-assets/reactive/launchpad-assets.py
index ee2b74e..8608197 100644
--- a/charm/launchpad-assets/reactive/launchpad-assets.py
+++ b/charm/launchpad-assets/reactive/launchpad-assets.py
@@ -37,15 +37,24 @@ def configure_convoy(config):
     # the payloads directory include the suffix.  However, the +combo URLs
     # generated by the appserver don't include the suffix, so the names of
     # symlinks in the convoy path shouldn't either.
-    link_path = convoy_path / f"rev{build_label.split('-', 1)[0]}"
-    if not link_path.is_symlink():
-        link_path.symlink_to(
+    current_link_path = convoy_path / f"rev{build_label.split('-', 1)[0]}"
+    if not current_link_path.is_symlink():
+        current_link_path.symlink_to(
             Path(base.payloads_dir()) / build_label / "build" / "js"
         )
     for link_path in convoy_path.iterdir():
         if link_path.name.startswith("rev") and link_path.is_symlink():
             if not link_path.exists():
                 link_path.unlink()
+    # In dev mode, the appserver generates combo URLs without a "rev..."
+    # path segment.  Make these work.
+    for js_name in ("lp", "yui"):
+        current_link_js_path = current_link_path / js_name
+        convoy_js_path = convoy_path / js_name
+        if convoy_js_path.is_symlink():
+            convoy_js_path.unlink()
+        if config["devmode"] and current_link_js_path.exists():
+            convoy_js_path.symlink_to(current_link_js_path)
 
     templating.render(
         "convoy.service.j2", "/lib/systemd/system/convoy.service", config