-
-
Notifications
You must be signed in to change notification settings - Fork 0
Added lagoon_logs.
#75
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
Conversation
📝 WalkthroughWalkthroughA new dependency on the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Lite
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
composer.json(1 hunks)config/default/core.extension.yml(1 hunks)config/default/lagoon_logs.settings.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: AlexSkrypnyk
PR: drevops/client-vu-cms#2
File: .lagoon.yml:18-21
Timestamp: 2025-05-29T06:57:43.517Z
Learning: Lagoon platform automatically handles masking or preventing exposure of sensitive environment variables in logs, so additional manual masking/unsetting of sensitive variables after debug output is not required in .lagoon.yml configurations.
📚 Learning: composer dependencies are automatically updated via renovatebot...
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-06T00:44:35.051Z
Learning: Composer dependencies are automatically updated via RenovateBot
Applied to files:
composer.json
📚 Learning: composer dependencies are updated automatically with compatibility checks via renovatebot...
Learnt from: CR
PR: drevops/website#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T16:58:49.534Z
Learning: Composer dependencies are updated automatically with compatibility checks via RenovateBot
Applied to files:
composer.json
📚 Learning: lagoon platform automatically handles masking or preventing exposure of sensitive environment variab...
Learnt from: AlexSkrypnyk
PR: drevops/client-vu-cms#2
File: .lagoon.yml:18-21
Timestamp: 2025-05-29T06:57:43.517Z
Learning: Lagoon platform automatically handles masking or preventing exposure of sensitive environment variables in logs, so additional manual masking/unsetting of sensitive variables after debug output is not required in .lagoon.yml configurations.
Applied to files:
config/default/lagoon_logs.settings.yml
🔇 Additional comments (2)
composer.json (1)
27-27: Verifydrupal/lagoon_logs3.x compatibility before locking it inFile: composer.json
Lines: 27–27Snippet showing the pinned version:
"drupal/lagoon_logs": "^3.0",I attempted to fetch Packagist metadata for all 3.x tags but ran into JSON‐indexing errors (likely due to underscore vs. hyphen in the package name or the wrong API endpoint). Please re‐run a corrected check to ensure every 3.x release declares compatibility with Drupal 11 and PHP 8.3 (earlier 3.x tags were Drupal 10 only). For example:
#!/usr/bin/env bash set -euo pipefail # 1. Packagist check for both naming variants for pkg in lagoon_logs lagoon-logs; do echo "Checking packagist.org/packages/drupal/$pkg.json …" curl -s "https://packagist.org/packages/drupal/$pkg.json" | jq -r ' select(.package) | .package.versions | to_entries[] | select(.key | test("^3\\.")) | "\(.key) → drupal/core: \(.value.require["drupal/core"] // "n/a"), php: \(.value.require.php // "n/a")"' \ | sort done # 2. Inspect composer.json on the 3.x branch echo -e "\ncomposer.json on branch 8.x-3.x (or 3.x):" for branch in 8.x-3.x 3.x; do echo "Branch: $branch" curl -s "https://raw.githubusercontent.com/drupal/lagoon_logs/$branch/composer.json" | jq -r ' if .require then "drupal/core: \(.require["drupal/core"] // "n/a")\nphp: \(.require.php // "n/a")" else "— composer.json not found on this branch" end' doneconfig/default/lagoon_logs.settings.yml (1)
1-2: Config hash looks auto-generated – no action needed.The
_core.default_config_hashvalue is correctly present; nothing to change.
| "drupal/gin": "^4.0.6", | ||
| "drupal/gin_toolbar": "^2", | ||
| "drupal/google_tag": "^2.0", | ||
| "drupal/lagoon_logs": "^3.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remember to commit the updated composer.lock.
composer.json changed but no composer.lock diff is present. Lock-file drift will break reproducible builds in CI.
🤖 Prompt for AI Agents
In composer.json at line 27, you updated the dependencies but did not commit the
corresponding composer.lock file. Run 'composer update' locally to regenerate
the composer.lock file reflecting the changes in composer.json, then commit the
updated composer.lock alongside composer.json to ensure consistent and
reproducible builds in CI.
| history: 0 | ||
| image: 0 | ||
| inline_form_errors: 0 | ||
| lagoon_logs: 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Module enabled for every install – is that intentional?
lagoon_logs is enabled in the default profile (value 0).
If the logging endpoint only exists on Lagoon, local/dev environments will spam connection errors. Consider moving the enable flag to a Lagoon-only config-split instead.
🤖 Prompt for AI Agents
In config/default/core.extension.yml at line 46, the lagoon_logs module is
enabled by default with a value of 0, causing connection errors in local/dev
environments where the logging endpoint does not exist. To fix this, remove or
disable lagoon_logs from the default profile and instead enable it only in a
Lagoon-specific config-split file to prevent errors outside the Lagoon
environment.
| host: application-logs.lagoon.svc | ||
| port: 5140 | ||
| identifier: drupal | ||
| disable: 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Hard-coded endpoint may block non-Lagoon environments.
host: application-logs.lagoon.svc and port: 5140 are cluster-internal.
To keep local and CI installs fast/failure-free, inject these via environment variables or an override split rather than shipping them in the default config.
🤖 Prompt for AI Agents
In config/default/lagoon_logs.settings.yml around lines 3 to 6, the host and
port values are hard-coded to cluster-internal endpoints, which can cause issues
in non-Lagoon environments. Modify the configuration to use environment
variables or an override mechanism to set the host and port dynamically,
allowing local and CI installs to run without failure by not relying on these
fixed internal addresses.
Summary by CodeRabbit