Fix path subfolder and inject more env#237
Conversation
|
Note Reviews pausedUse the following commands to manage reviews:
📝 WalkthroughWalkthroughDocumentation in Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
🪧 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/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
inject.sh (1)
28-30: ASSET_URL injection added — looks good; consider escaping edge cases.LGTM to support subfolder setups. Minor: replace_or_insert uses sed replacement where unescaped
&or backslashes in values can misbehave. ASSET_URL usually won’t include&, but it’s easy to make this robust.You could harden replace_or_insert to escape sed replacement chars:
# Example helper for robust escaping (outside current hunk) escape_sed_replacement() { printf '%s' "$1" | sed -e 's/[\/&]/\\&/g' }Then call with:
val_escaped="$(escape_sed_replacement "$ASSET_URL")" replace_or_insert "ASSET_URL" "$val_escaped"docker-compose.yml (1)
80-84: Comment updates for subfolder usage are helpful; trim trailing spaces on ASSET_URL line.Good clarification about APP_DIR/ASSET_URL. YAMLlint flagged trailing spaces on Line 83.
Apply:
- #- ASSET_URL= + #- ASSET_URL=.env.example (1)
21-29: Subfolder guidance is clear; minor consistency tweaks suggested.Small copy edits can improve clarity and consistency:
- Capitalize TLD.
- Use https consistently in examples (you already do above).
- Add a note to avoid a trailing slash in APP_DIR.
-# If using Lychee in a sub folder, specify the path after the tld here. +# If using Lychee in a sub folder, specify the path after the TLD here. @@ -# and APP_DIR=/path/to/lychee +# and APP_DIR=/path/to/lychee # no trailing slash @@ -# ASSET_URL=http://example.com/path/to/lychee +# ASSET_URL=https://example.com/path/to/lychee
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.env.example(2 hunks)docker-compose.yml(2 hunks)inject.sh(2 hunks)
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 51-51: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🪛 YAMLlint (1.37.1)
docker-compose.yml
[error] 83-83: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
inject.sh (1)
331-337: New auth toggles injection is consistent with docker-compose and docs.DISABLE_BASIC_AUTH and DISABLE_WEBAUTHN are correctly passed through to /conf/.env when set. Placement near VITE_HTTP_PROXY_ENABLED keeps related toggles grouped.
docker-compose.yml (1)
128-129: Expose DISABLE_ toggles — LGTM and consistent with inject.sh/.env.example.*Both variables default to false and are injected by inject.sh when set.
|
@coderabbitai ignore |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/test_pull.yml (2)
35-48: Use github.rest. namespace for Octokit REST calls in github-script*actions/github-script v7 exposes REST methods under github.rest.actions., not github.actions.. Using the old namespace can break on version bumps or in stricter runtimes.
Apply this diff within the script block:
- var artifacts = await github.actions.listWorkflowRunArtifacts({ + var artifacts = await github.rest.actions.listWorkflowRunArtifacts({ owner: context.repo.owner, repo: context.repo.repo, run_id: ${{ github.event.workflow_run.id }}, }); var matchArtifact = artifacts.data.artifacts.filter((artifact) => { return artifact.name == "artifact" })[0]; - var download = await github.actions.downloadArtifact({ + var download = await github.rest.actions.downloadArtifact({ owner: context.repo.owner, repo: context.repo.repo, artifact_id: matchArtifact.id, archive_format: 'zip', });Optionally, harden the script to fail fast if the artifact is missing:
var matchArtifact = artifacts.data.artifacts.filter((artifact) => { return artifact.name == "artifact" })[0]; + if (!matchArtifact) { + core.setFailed('No artifact named "artifact" found for the triggering run.'); + return; + }
201-203: YAML bug: invalid run block; use a scalar block or inline shellAs written, this is not valid YAML for a run step and will be ignored/parsed incorrectly by Actions.
Apply one of these minimal fixes:
- run: - echo "DB_PASSWORD=password" >> .env + run: echo "DB_PASSWORD=password" >> .envor, if you plan to add more lines later:
- run: - echo "DB_PASSWORD=password" >> .env + run: | + echo "DB_PASSWORD=password" >> .env
🧹 Nitpick comments (3)
.github/workflows/test_pull.yml (3)
213-215: Pin the crane image to a version or digest instead of the floating debug tagUsing a floating tag can yield non-reproducible builds and supply-chain risk.
For example:
- container: - image: gcr.io/go-containerregistry/crane:debug + container: + image: gcr.io/go-containerregistry/crane:v0.19.1Even better, pin to a digest if possible.
80-91: Double-check that the imported image carries the expected tag before pushThe push assumes the tar contains lycheeorg/lychee:testing-${{ github.event.workflow_run.id }}. If tags differ, the push will fail. Consider retagging after load to be explicit.
- name: "Import image" run: docker image load -i ./lychee.tar + - name: "Ensure expected tag" + run: | + IMG_ID=$(docker images --format '{{.Repository}}:{{.Tag}} {{.ID}}' | awk '$1 ~ /lycheeorg\/lychee/ {print $2}' | head -n1) + docker tag "$IMG_ID" "lycheeorg/lychee:testing-${{ github.event.workflow_run.id }}"
92-186: Optional: add timeouts to curl checks to avoid long hangsA short --max-time/--connect-timeout helps jobs fail fast when services don’t come up.
For example:
- run: 'curl -sSw "%{stderr}%{http_code}" http://localhost/ > /dev/null && curl -f http://localhost/' + run: 'curl --connect-timeout 5 --max-time 20 -sSw "%{stderr}%{http_code}" http://localhost/ > /dev/null && curl --connect-timeout 5 --max-time 20 -f http://localhost/'Apply similarly to other GET steps.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/test_pull.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
.github/workflows/test_pull.yml (2)
34-34: Nice touch: added explicit artifact download logThis improves traceability when debugging cross-workflow artifact transfers.
52-90: Summary mismatch: steps appear present here but were described as removedThe AI summary mentions removal of Unzip/Get PR/Create check/debug/Import/Login/Push steps, but they are present in this file (Lines 52–90). Confirm whether they should remain, particularly the Docker Hub push on PR-triggered runs.
If the intent was to drop these, remove the steps and adjust downstream jobs (which rely on the pushed tag) accordingly, or keep them intentionally. I can propose a consistent cut if you confirm the desired behavior.
✅ Actions performedReviews paused. |
ildyria
force-pushed
the
fix-path-subfolder
branch
from
39f4aca to
63a0fd5
Compare
1 participant
Summary by CodeRabbit
New Features
Documentation
Chores