Skip to content

Use EDGE_APP_ID for stage and production deploys - #17

Merged
nicomiguelino merged 4 commits into
developmentfrom
feature/edge-app-id-env-var
Sep 3, 2026
Merged

Use EDGE_APP_ID for stage and production deploys#17
nicomiguelino merged 4 commits into
developmentfrom
feature/edge-app-id-env-var

Conversation

@nicomiguelino

@nicomiguelino nicomiguelino commented Sep 3, 2026

Copy link
Copy Markdown
Contributor
  • Set STAGE_EDGE_APP_ID and PRODUCTION_EDGE_APP_ID repo variables and pass them through to the initialize/update actions as edge_app_id. An EDGE_APP_ID environment secret takes precedence when set, with these variables as the fallback until one exists, so switching to Environment secrets later needs no workflow changes.
  • Dropped id from screenly.yml and deleted screenly_qc.yml since a single manifest is now used regardless of environment.
  • Pinned edge-apps-actions to a commit for now, since v26.9.0 can't be tagged there until a repo ruleset is adjusted. Will switch to the tag once it exists.

Set STAGE_EDGE_APP_ID and PRODUCTION_EDGE_APP_ID repo variables and
pass them through to the initialize/update actions as edge_app_id,
falling back from an EDGE_APP_ID environment secret so this upgrades
cleanly once one is set.

Drop the id from screenly.yml and delete screenly_qc.yml since a
single manifest is now used regardless of environment. Pin the
edge-apps-actions ref to a commit until v26.9.0 can be tagged.
Copilot AI lite review requested due to automatic review settings September 3, 2026 21:40

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The production initialization workflow can incorrectly fall back to the stage Edge App ID if the production variable is unset, risking operations against the wrong environment.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Updates the Edge App deployment/initialization workflows to use environment-specific Edge App IDs (stage vs production) passed into Screenly’s GitHub Actions, while removing hardcoded IDs from manifests to allow a single manifest across environments.

Changes:

  • Removed the id from screenly.yml and deleted screenly_qc.yml to standardize on one manifest.
  • Updated GitHub workflows to pass edge_app_id from repo variables (with a secret fallback) and pinned edge-apps-actions to a commit.
  • Added local .env support (.env.example) and ignored .env in git.
File summaries
File Description
screenly.yml Removes the hardcoded manifest id to enable a single manifest across environments.
screenly_qc.yml Deleted QC-specific manifest file as it’s no longer needed.
.gitignore Ignores .env to prevent committing local secrets/config.
.github/workflows/update-edge-app.yml Passes stage/production edge_app_id via vars/secrets and pins the action revision.
.github/workflows/initialize-edge-app.yml Adds edge_app_id wiring during initialization and pins the action revision.
.env.example Documents local EDGE_APP_ID override for CLI usage.
Review details
  • Files reviewed: 5/6 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/initialize-edge-app.yml Outdated
Comment thread .env.example Outdated
…nv.example

- Only fall back to the stage variable when the target environment is stage
- Remove .env.example since the CLI doesn't load .env files
Copilot AI review requested due to automatic review settings September 3, 2026 22:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The edge_app_id fallback order in the workflows contradicts the PR’s stated migration path and can select the wrong ID when both secret and variables are present.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 4/5 changed files
  • Comments generated: 3
  • Review effort level: Lite

Comment thread .github/workflows/initialize-edge-app.yml
Comment thread .github/workflows/update-edge-app.yml
Comment thread .github/workflows/update-edge-app.yml
Copilot AI review requested due to automatic review settings September 3, 2026 22:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

It introduces a visible temporary UI marker in index.html and the initialize workflow’s edge_app_id expression can evaluate to boolean false instead of a string.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

.github/workflows/initialize-edge-app.yml:41

  • This expression can evaluate to the boolean false (e.g., if the selected environment's repo variable is unset/empty), which would pass a non-string edge_app_id to the action. Add a final fallback (e.g., || '') so the value is always a string and avoid accidental type surprises.
  • Files reviewed: 5/6 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread index.html Outdated
Copilot AI review requested due to automatic review settings September 3, 2026 22:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The workflow and manifest updates align with the described deployment strategy, with only a minor readability nit in one expression.

Review details

Suppressed comments (1)

.github/workflows/initialize-edge-app.yml:41

  • The edge_app_id expression works, but the nested parentheses make it harder to read and reason about. You can simplify it without changing the precedence/safety (it will still resolve to false if the environment-specific var is missing for the selected environment).
          edge_app_id: ${{ secrets.EDGE_APP_ID || (inputs.environment == 'production' && vars.PRODUCTION_EDGE_APP_ID || (inputs.environment == 'stage' && vars.STAGE_EDGE_APP_ID)) }}
  • Files reviewed: 4/5 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants