build eloqstore release tarball#333
Conversation
WalkthroughAdds ELOQDSS_ELOQSTORE support across Concourse pipelines and tasks, build scripts, artifacts, and CMake: new git/registry resources and get steps, new build tasks and inputs, build_tarball symlink and DATA_STORE_TYPE mapping, artifact configs, submodule pointer bump, and minor tagging script updates. Changes
Sequence Diagram(s)sequenceDiagram
rect rgb(250,250,245)
participant Concourse
participant GitRepo as Git
participant Worker
participant Script as build_tarball.bash
note over Concourse,GitRepo: Pipeline fetches resources (new eloqstore_src/_main)
end
Concourse->>GitRepo: get `eloqstore_src[_main]`
Concourse->>Worker: run task `build-eloqdss-eloqstore-type` (params incl. DATA_STORE_TYPE=ELOQDSS_ELOQSTORE)
Worker->>Worker: assemble inputs (includes `eloqstore_src`)
Worker->>Script: invoke build_tarball.bash with DATA_STORE_TYPE=ELOQDSS_ELOQSTORE
Script->>Worker: create symlink eloq_data_store_service/eloqstore -> eloqstore_src
Script->>Script: map DATA_STORE_TYPE -> eloqdss_eloqstore and set WITH_LOG_STATE
Worker->>Concourse: task success / failure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
concourse/scripts/build_tarball.bash (1)
113-114: Update error messages to include ELOQDSS_ELOQSTORE.The validation error messages at lines 113 and 127 list only ELOQDSS_ROCKSDB_CLOUD_S3, ELOQDSS_ROCKSDB_CLOUD_GCS, and ELOQDSS_ROCKSDB, but the code now supports ELOQDSS_ELOQSTORE. These messages should be updated for consistency and clarity.
Apply this diff to fix the error messages:
- echo "DATA_STORE_TYPE must be provided. Supported: ELOQDSS_ROCKSDB_CLOUD_S3, ELOQDSS_ROCKSDB_CLOUD_GCS, ELOQDSS_ROCKSDB" + echo "DATA_STORE_TYPE must be provided. Supported: ELOQDSS_ROCKSDB_CLOUD_S3, ELOQDSS_ROCKSDB_CLOUD_GCS, ELOQDSS_ROCKSDB, ELOQDSS_ELOQSTORE"- echo "Unsupported DATA_STORE_TYPE: ${DATA_STORE_TYPE}. Supported: ELOQDSS_ROCKSDB_CLOUD_S3, ELOQDSS_ROCKSDB_CLOUD_GCS, ELOQDSS_ROCKSDB" + echo "Unsupported DATA_STORE_TYPE: ${DATA_STORE_TYPE}. Supported: ELOQDSS_ROCKSDB_CLOUD_S3, ELOQDSS_ROCKSDB_CLOUD_GCS, ELOQDSS_ROCKSDB, ELOQDSS_ELOQSTORE"Also applies to: 127-128
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
concourse/pipeline/build_debug_tarball.yml(2 hunks)concourse/pipeline/build_nightly_tarball.yml(3 hunks)concourse/pipeline/build_release_tarball.yml(3 hunks)concourse/scripts/build_tarball.bash(2 hunks)concourse/tasks/build_debug_tarball.yml(1 hunks)concourse/tasks/build_nightly_tarball.yml(1 hunks)concourse/tasks/build_release_tarball.yml(1 hunks)src/mongo/db/modules/eloq/store_handler(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: githubzilla
Repo: eloqdata/eloqdoc PR: 211
File: src/mongo/db/modules/eloq/cmake/build_eloq_log_service.cmake:26-80
Timestamp: 2025-09-25T12:24:06.434Z
Learning: The user githubzilla implemented a fix in commit fe98aaf to address the MEMORY state incorrectly triggering RocksDB discovery in build_eloq_log_service.cmake. This was a control flow issue where MEMORY builds were falling into an else branch that still attempted RocksDB discovery.
📚 Learning: 2025-09-25T11:58:50.446Z
Learnt from: githubzilla
Repo: eloqdata/eloqdoc PR: 211
File: src/mongo/db/modules/eloq/cmake/build_log_service.cmake:116-119
Timestamp: 2025-09-25T11:58:50.446Z
Learning: The build_log_service.cmake file is specifically for "open log" functionality and only supports the ROCKSDB log state type (LOG_STATE_TYPE_RKDB). The full log state configuration with cloud variants (ROCKSDB_CLOUD_S3, ROCKSDB_CLOUD_GCS) is handled in build_eloq_log_service.cmake.
Applied to files:
concourse/pipeline/build_debug_tarball.yml
🪛 YAMLlint (1.37.1)
concourse/pipeline/build_debug_tarball.yml
[error] 17-17: syntax error: mapping values are not allowed here
(syntax)
🔇 Additional comments (16)
src/mongo/db/modules/eloq/store_handler (1)
1-1: Submodule pointer update verified—commit is valid and aligns with PR objectives.The new commit hash
c70ebbf5is valid in the eloqdata/store_handler repository and represents a legitimate performance optimization ("parallelize write requests in upsert ranges" #150, dated 2025-11-21). The update from commit #148 to #150 improves batched-dispatch flow and metadata batching for data store upsert operations. This aligns with the PR's broader objective of enabling ELOQDSS_ELOQSTORE support across the build pipeline and codebase, with no breaking changes evident.concourse/tasks/build_debug_tarball.yml (1)
8-8: Input addition aligns with pipeline changes.The eloqstore_src input is properly declared and will receive the source from the updated pipeline resource.
concourse/tasks/build_release_tarball.yml (1)
8-8: Input addition aligns with other tarball task definitions.The eloqstore_src input correctly matches the declaration in build_debug_tarball.yml and build_nightly_tarball.yml for consistency across all tarball build workflows.
concourse/tasks/build_nightly_tarball.yml (1)
8-8: Input addition is consistent with other tarball tasks.Matches the eloqstore_src input added to build_debug_tarball.yml and build_release_tarball.yml.
concourse/scripts/build_tarball.bash (3)
30-34: Symlink creation follows established patterns.The conditional handling for ELOQDSS_ELOQSTORE correctly mirrors the symlink pattern used for raft_host_manager. No issues detected.
124-125: DATA_STORE_ID mapping is consistent with existing patterns.The new conditional correctly adds eloqdss_eloqstore to the DATA_STORE_TYPE validation chain.
202-209: Verify WITH_LOG_STATE handling for ELOQDSS_ELOQSTORE.The code currently defaults to
export WITH_LOG_STATE=ROCKSDBfor ELOQDSS_ELOQSTORE (line 208), since it falls through the ROCKSDB_CLOUD_S3/GCS conditionals. Confirm whether this is the intended behavior or if eloqstore requires a different log state configuration.concourse/pipeline/build_debug_tarball.yml (3)
9-15: eloqstore_src_main resource is properly defined.The git resource correctly points to the eloqstore repository and follows the same pattern as other resources in the pipeline.
52-53: eloqstore_src fetch is correctly configured.The resource is properly fetched and aliased within the in_parallel steps for the ubuntu2204-main plan.
57-72: build-eloqdss-eloqstore-type task is properly configured.The task definition includes all necessary parameters and correctly references the build_debug_tarball.yml task file. Configuration is consistent with other build tasks in the pipeline.
concourse/pipeline/build_nightly_tarball.yml (4)
29-34: eloqstore_src resource is properly defined.The git resource correctly integrates into the nightly build pipeline with appropriate branching and access configuration.
127-127: eloqstore_src fetches are correctly added to both plans.The resource is properly retrieved in both ubuntu2204 and ubuntu2404 build plans, enabling eloqstore-based builds across variants.
Also applies to: 202-202
128-142: ubuntu2204 build-eloqdss-eloqstore-type task is properly configured.The task correctly specifies the new eloqstore build variant with appropriate parameters for nightly builds.
204-218: ubuntu2404 eloqdss-eloqstore task is properly configured.The task correctly builds the eloqstore variant for ubuntu2404 with appropriate parameters and resource dependencies.
concourse/pipeline/build_release_tarball.yml (2)
10-15: eloqstore_src_main resource is properly defined.The git resource correctly integrates into the release build pipeline with appropriate access configuration, matching the resource definitions in debug and nightly pipelines.
121-122: eloqstore_src fetches and build tasks are properly configured for release builds.Both ubuntu2204 and ubuntu2404 build variants correctly fetch eloqstore_src and execute the build-eloqdss-eloqstore-type task with appropriate parameters. The ubuntu2404 variant appropriately includes ELOQ_MODULE_ENABLED, matching other ubuntu2404 tasks in the pipeline.
Also applies to: 124-137, 184-201
| - name: image_ubuntu2404-main | ||
| type: registry-image | ||
| source: | ||
| repository: eloqdata/eloq-dev-ci-ubuntu2404 | ||
|
|
There was a problem hiding this comment.
Fix YAML indentation syntax error in image_ubuntu2404-main resource.
The resource definition has incorrect indentation. The properties (type, source) should be indented 2 spaces from the dash, not 4. This causes a YAML parsing error and will prevent the pipeline from loading.
Apply this diff to fix the indentation:
- name: image_ubuntu2404-main
- type: registry-image
- source:
- repository: eloqdata/eloq-dev-ci-ubuntu2404
+ name: image_ubuntu2404-main
+ type: registry-image
+ source:
+ repository: eloqdata/eloq-dev-ci-ubuntu2404📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: image_ubuntu2404-main | |
| type: registry-image | |
| source: | |
| repository: eloqdata/eloq-dev-ci-ubuntu2404 | |
| - name: image_ubuntu2404-main | |
| type: registry-image | |
| source: | |
| repository: eloqdata/eloq-dev-ci-ubuntu2404 |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 17-17: syntax error: mapping values are not allowed here
(syntax)
🤖 Prompt for AI Agents
In concourse/pipeline/build_debug_tarball.yml around lines 16 to 20 the resource
"image_ubuntu2404-main" has incorrect YAML indentation (properties `type` and
`source` are indented 4 spaces instead of 2), which breaks parsing; fix it by
aligning `type:` and `source:` exactly two spaces under the resource dash (so
they are children of the resource name), and ensure their nested keys (like
`repository:`) remain indented consistently under `source:`.
735eef4 to
2d7cd42
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
concourse/artifact/ELOQDSS_ELOQSTORE/eloqdoc.conf(1 hunks)concourse/artifact/ELOQDSS_ELOQSTORE/rclone.ini(1 hunks)concourse/scripts/git-tag.sh(1 hunks)concourse/scripts/tag.sh(1 hunks)src/mongo/db/modules/eloq/store_handler(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/mongo/db/modules/eloq/store_handler
🚧 Files skipped from review as they are similar to previous changes (3)
- concourse/scripts/tag.sh
- concourse/scripts/git-tag.sh
- concourse/artifact/ELOQDSS_ELOQSTORE/eloqdoc.conf
🔇 Additional comments (1)
concourse/artifact/ELOQDSS_ELOQSTORE/rclone.ini (1)
4-4: The review comment is based on an incorrect assumption about GCS project identifier format.rclone's
project_numberfield accepts a string containing either the numeric project number or the alphanumeric project ID. The valueeloqdevis a valid GCS Project ID format and requires no changes. The comment should be dismissed.Likely an incorrect or invalid review comment.
| project_number = eloqdev | ||
| # anonymous access to the bucket. | ||
| anonymous = false | ||
| bucket_acl = |
There was a problem hiding this comment.
🧩 Analysis chain
Use environment variables for file paths and secrets.
Hardcoding the service account file path (/home/eloq/workspace/iam.json) reduces portability and increases security risks. This path assumes a specific user and filesystem layout, which may not hold across different deployment environments or CI/CD systems.
Additionally, service account files containing credentials should not be committed to version control. They should be provided at runtime via environment variables or mounted secrets.
Consider these improvements:
- Use an environment variable for the service account path (e.g.,
service_account_file = ${ELOQSTORE_SERVICE_ACCOUNT_FILE}or rclone's interpolation syntax if supported). - Document how the service account file should be provisioned and mounted in the Concourse pipeline.
- Ensure the service account file is never committed to the repository.
Please verify:
- How the service account file is currently provisioned in the deployment environment.
- Whether rclone supports environment variable interpolation in its configuration.
- Whether this path is intended to be used in all environments (dev, staging, production).
Also applies to: 15-15
I need to verify the actual content of the rclone.ini file and check rclone's capabilities. Let me gather this information.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find and display the full rclone.ini file with line numbers
find . -name "rclone.ini" -type f 2>/dev/null | head -20
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Display the rclone.ini file content if it exists
if [ -f "concourse/artifact/ELOQDSS_ELOQSTORE/rclone.ini" ]; then
cat -n "concourse/artifact/ELOQDSS_ELOQSTORE/rclone.ini"
fiLength of output: 602
🌐 Web query:
rclone configuration environment variable interpolation support
💡 Result:
Short answer: rclone does not support shell-style environment-variable interpolation inside rclone.conf (e.g. ${FOO} in the file). Use rclone's documented environment-variable mechanisms instead.
Details and how to do it:
- rclone supports configuring any option or config entry via environment variables named RCLONE_ or RCLONE_CONFIG__. Use those to supply secrets/overrides rather than embedding ${VAR} in rclone.conf. [1]
- Historical/forum threads confirm there is no built‑in syntax for “read this config value from a named env var” inside the conf file; the recommended approach is the per-remote RCLONE_CONFIG_... env vars (or backend-specific env_auth modes). [2][3]
Examples:
- Export a remote value: export RCLONE_CONFIG_MYS3_ACCESS_KEY_ID=XXX and export RCLONE_CONFIG_MYS3_SECRET_ACCESS_KEY=YYY then use mys3: as a remote. [1]
Sources:
[1] rclone docs — Environment variables (RCLONE_ and RCLONE_CONFIG_ naming).
[2] rclone forum discussion about setting config values from environment variables / env_auth.
[3] rclone forum / GitHub issues about env-vars vs config visibility and RC GUI behavior.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check if iam.json exists in the repository
find . -name "iam.json" -type f 2>/dev/null
Length of output: 0
Fix hardcoded service account path using rclone's environment variable mechanism.
The hardcoded path service_account_file = /home/eloq/workspace/iam.json at line 15 is a critical security and portability risk. However, rclone does not support shell-style ${VAR} interpolation in its configuration file.
Instead, rclone supports configuring any option via environment variables named RCLONE_CONFIG_<REMOTENAME>_<OPTION>. For this remote, use:
RCLONE_CONFIG_ELOQSTORE_CLOUD_SERVICE_ACCOUNT_FILE=/path/to/iam.json
Remove the service_account_file line from rclone.ini (line 15) or leave it empty, and provision the environment variable at runtime in your Concourse pipeline. This ensures:
- The path is not hardcoded and can vary per environment (dev, staging, production)
- Credentials are provisioned securely at runtime
- The configuration file remains portable across deployments
🤖 Prompt for AI Agents
In concourse/artifact/ELOQDSS_ELOQSTORE/rclone.ini (referencing lines ~7 and the
service_account_file at line 15), remove the hardcoded service_account_file =
/home/eloq/workspace/iam.json (or leave it empty) and instead provision the
credential path via an environment variable when the pipeline runs; set
RCLONE_CONFIG_ELOQSTORE_CLOUD_SERVICE_ACCOUNT_FILE=/path/to/iam.json in the
Concourse job/task environment or credentials management and ensure the
rclone.ini no longer contains the hardcoded path so the remote reads the service
account file from that env var at runtime.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
concourse/pipeline/build_release_tarball.yml (2)
10-15: Add eloqstore resource to ubuntu2004, centos7, centos8, and rocky9 jobs for consistency.The eloqstore_src resource is fetched only in ubuntu2204 and ubuntu2404 jobs, but not in ubuntu2004, centos7, centos8, and rocky9 jobs. If eloqstore support should eventually be extended to these OS variants, consider adding eloqstore_src as a get step in their plans to prevent future rework.
Otherwise, if eloqstore is genuinely unsupported on these platforms, document this decision to clarify the platform-specific limitation.
124-137: Consider enabling ELOQ_MODULE_ENABLED for ubuntu2204 eloqstore build.The ubuntu2204 build-eloqdss-eloqstore-type task omits
ELOQ_MODULE_ENABLED: "true", while the ubuntu2404 equivalent (lines 187–201) includes it. For consistency and feature parity, consider adding this parameter to the ubuntu2204 eloqstore task.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
concourse/artifact/ELOQDSS_ELOQSTORE/eloqdoc.conf(1 hunks)concourse/artifact/ELOQDSS_ELOQSTORE/rclone.ini(1 hunks)concourse/pipeline/build_debug_tarball.yml(5 hunks)concourse/pipeline/build_nightly_tarball.yml(9 hunks)concourse/pipeline/build_release_tarball.yml(3 hunks)concourse/scripts/build_tarball.bash(3 hunks)concourse/scripts/git-tag.sh(1 hunks)concourse/scripts/tag.sh(1 hunks)concourse/tasks/build_debug_tarball.yml(1 hunks)concourse/tasks/build_nightly_tarball.yml(1 hunks)concourse/tasks/build_release_tarball.yml(1 hunks)src/mongo/db/modules/eloq/CMakeLists.txt(1 hunks)src/mongo/db/modules/eloq/store_handler(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- concourse/tasks/build_nightly_tarball.yml
- concourse/scripts/tag.sh
- concourse/tasks/build_debug_tarball.yml
- concourse/scripts/git-tag.sh
- src/mongo/db/modules/eloq/store_handler
- concourse/artifact/ELOQDSS_ELOQSTORE/eloqdoc.conf
- concourse/artifact/ELOQDSS_ELOQSTORE/rclone.ini
- concourse/tasks/build_release_tarball.yml
- concourse/scripts/build_tarball.bash
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-25T12:24:06.434Z
Learnt from: githubzilla
Repo: eloqdata/eloqdoc PR: 211
File: src/mongo/db/modules/eloq/cmake/build_eloq_log_service.cmake:26-80
Timestamp: 2025-09-25T12:24:06.434Z
Learning: The user githubzilla implemented a fix in commit fe98aaf to address the MEMORY state incorrectly triggering RocksDB discovery in build_eloq_log_service.cmake. This was a control flow issue where MEMORY builds were falling into an else branch that still attempted RocksDB discovery.
Applied to files:
src/mongo/db/modules/eloq/CMakeLists.txt
📚 Learning: 2025-09-25T11:58:50.446Z
Learnt from: githubzilla
Repo: eloqdata/eloqdoc PR: 211
File: src/mongo/db/modules/eloq/cmake/build_log_service.cmake:116-119
Timestamp: 2025-09-25T11:58:50.446Z
Learning: The build_log_service.cmake file is specifically for "open log" functionality and only supports the ROCKSDB log state type (LOG_STATE_TYPE_RKDB). The full log state configuration with cloud variants (ROCKSDB_CLOUD_S3, ROCKSDB_CLOUD_GCS) is handled in build_eloq_log_service.cmake.
Applied to files:
concourse/pipeline/build_debug_tarball.yml
🔇 Additional comments (6)
src/mongo/db/modules/eloq/CMakeLists.txt (2)
51-51: Clarify ELOQDSS_ELOQSTORE addition on line 51.The AI summary mentions adding ELOQDSS_ELOQSTORE support, but line 51 shows
"ELOQDSS_ELOQSTORE"in theset_propertystrings without a change marker (~). This suggests either:
- ELOQDSS_ELOQSTORE was already present in the previous version
- The change annotation is incomplete
Please clarify whether line 51 was modified to add ELOQDSS_ELOQSTORE as a valid
WITH_DATA_STOREoption in this PR, or if that support was already present. If it was added in this PR, it should be marked with~.Based on learnings, ensure that the control flow for different data store types (especially ELOQDSS_ELOQSTORE) is correctly handled in
cmake/build_data_store.cmakeand related files, similar to how the prior fix addressed MEMORY state control flow issues.
23-26: I'll help you verify the concerns raised in this review comment about the CMakeLists.txt file changes. Let me investigate the codebase to validate the claims.
<function_calls>
#!/bin/bashFirst, let's view the CMakeLists.txt file to understand the full context
cat -n src/mongo/db/modules/eloq/CMakeLists.txt 2>/dev/null | head -60
</function_calls>Now let me search for ELOQ_MODULE_ENABLED references in pipeline and build configuration files:
<function_calls>
#!/bin/bashSearch for ELOQ_MODULE_ENABLED in YAML, JSON, and build config files
echo "=== Searching for ELOQ_MODULE_ENABLED in CI/pipeline configs ==="
rg -n 'ELOQ_MODULE_ENABLED' --type yaml --type json 2>/dev/null | head -30echo ""
echo "=== Searching for ELOQ_MODULE_ENABLED in all files ==="
rg -n 'ELOQ_MODULE_ENABLED' -C 2 2>/dev/null | head -50
</function_calls>Let me also check for the specific file and look for Concourse pipeline definitions:
<function_calls>
#!/bin/bashFind Concourse pipeline files and build configuration files
echo "=== Finding Concourse pipeline files ==="
find . -name ".yml" -o -name ".yaml" | grep -i concourse | head -20echo ""
echo "=== Finding build config files ==="
fd -e "yml" -e "yaml" -e "json" | grep -E "(pipeline|build|config)" | head -30echo ""
echo "=== Searching for references to ELOQ in all CMakeLists.txt files ==="
rg -n 'ELOQ' src/mongo/db/modules/eloq/CMakeLists.txt 2>/dev/null || echo "File not found or empty search"
</function_calls>Let me check if the file path is correct and search more broadly:
<function_calls>
#!/bin/bashCheck the repository structure
echo "=== Checking for eloq module ==="
find . -type d -name "eloq" 2>/dev/nullecho ""
echo "=== Looking for CMakeLists.txt files ==="
find . -name "CMakeLists.txt" -path "eloq" 2>/dev/nullecho ""
echo "=== List files in repo root to understand structure ==="
ls -la | head -20
</function_calls><function_calls>
#!/bin/bashCheck git status to see what branch we're on and what files are modified
echo "=== Git status and recent commits ==="
git status 2>/dev/null | head -20echo ""
echo "=== Recently modified files related to eloq ==="
git diff --name-only --cached 2>/dev/null | grep -i eloq || echo "No staged changes with eloq"echo ""
echo "=== All modified files (staged and unstaged) ==="
git diff --name-only 2>/dev/null | grep -i eloq || echo "No unstaged changes with eloq"echo ""
echo "=== Check for the specific file ==="
test -f "src/mongo/db/modules/eloq/CMakeLists.txt" && echo "File exists" || echo "File does not exist"
</function_calls><function_calls>
#!/bin/bashGet general repository info
echo "=== Repository info ==="
git log --oneline -1 2>/dev/null
git branch 2>/dev/nullecho ""
echo "=== List all files in src/mongo/db/modules if it exists ==="
ls -la src/mongo/db/modules 2>/dev/null || echo "Path does not exist"echo ""
echo "=== Check git ls-files for eloq ==="
git ls-files 2>/dev/null | grep -i eloq || echo "No eloq files in repository"
</function_calls>Based on my investigation, I need to clarify the situation:
The repository does not contain the file
src/mongo/db/modules/eloq/CMakeLists.txtthat the review comment references. Without access to the actual code snippet, CMakeLists.txt content, and related pipeline/configuration files, I cannot verify the concerns raised in the original review comment.However, I can still provide guidance based on the concerns identified:
Unable to verify the review comment due to missing file context.
The original review comment identifies potentially important concerns about changing
ELOQ_MODULE_ENABLEDfrom OFF to ON by default, including:
- Pipeline coordination impact
- Potential breaking changes for builds assuming the OFF default
- Discrepancies in change markers for line 51
To properly verify these concerns, I would need:
- Access to the actual
src/mongo/db/modules/eloq/CMakeLists.txtfile- The full context of the pull request showing what actually changed
- Any relevant Concourse pipeline or CI configuration files
- Test configurations that might be affected
concourse/pipeline/build_release_tarball.yml (1)
187-201: Verify that the task definition accepts the eloqstore_src input.Ensure that
eloqdoc_src/concourse/tasks/build_release_tarball.ymldeclareseloqstore_srcas an input; otherwise, the get step at line 184–185 will be unused and the task may fail if it tries to reference eloqstore sources.concourse/pipeline/build_debug_tarball.yml (2)
52-120: LGTM: Consistent ELOQ_MODULE_ENABLED propagation for debug builds.All ubuntu2204 debug build tasks consistently include
ELOQ_MODULE_ENABLED: "true", including the new eloqstore variant. This ensures the ELOQ module is enabled uniformly across debug build types.
9-14: Based on my verification, I can now provide the rewritten review comment:
The referenced resource
image_ubuntu2404-maindoes not exist in this file; verify you are reviewing the correct file.The file
concourse/pipeline/build_debug_tarball.ymlcontains onlyimage_ubuntu2204_main(lines 16-19), notimage_ubuntu2404-main. The resource definition at lines 9-14 (eloqstore_src_main) has correct YAML indentation. If you intended to flag indentation issues inbuild_release_tarball.ymlorbuild_tarball_open.yml(which do containimage_ubuntu2404_main), please clarify which file needs review.Likely an incorrect or invalid review comment.
concourse/pipeline/build_nightly_tarball.yml (1)
130-145: Verify task definition declares eloqstore_src as an input.Ensure that
eloqdoc_src/concourse/tasks/build_nightly_tarball.ymlincludeseloqstore_srcas an input; otherwise, the get step at line 129 will be unused and the task may fail if it requires eloqstore sources.
| - name: eloqstore_src | ||
| type: git | ||
| source: | ||
| branch: main | ||
| uri: git@github.com:eloqdata/eloqstore.git | ||
| private_key: ((git-key)) |
There was a problem hiding this comment.
Fix resource naming inconsistency: use eloqstore_src_main for consistency with other pipelines.
This file defines the eloqstore resource as eloqstore_src, but build_release_tarball.yml and build_debug_tarball.yml use eloqstore_src_main. This naming mismatch could cause confusion and maintenance issues. Standardize on eloqstore_src_main to align with other pipelines in the codebase.
Apply this diff:
- - name: eloqstore_src
+ - name: eloqstore_src_main
type: git
source:
branch: main
uri: git@github.com:eloqdata/eloqstore.git
private_key: ((git-key))Then update the get steps at lines 129 and 207 to reference eloqstore_src_main:
- - get: eloqstore_src
+ - get: eloqstore_src
+ resource: eloqstore_src_mainCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In concourse/pipeline/build_nightly_tarball.yml around lines 29 to 34, the git
resource is named eloqstore_src but other pipelines use eloqstore_src_main;
rename the resource to eloqstore_src_main and then update the two get steps
referenced at lines 129 and 207 to use eloqstore_src_main instead of
eloqstore_src so names match across pipelines and avoid confusion.
| - task: eloqdss-eloqstore | ||
| image: image_ubuntu2404 | ||
| params: | ||
| DATA_STORE_TYPE: "ELOQDSS_ELOQSTORE" | ||
| BUILD_LOG_SRV: "true" | ||
| GIT_SSH_KEY: ((git-key)) | ||
| ASAN: "OFF" | ||
| TAGGED: "" | ||
| BUILD_TYPE: RelWithDebInfo | ||
| ELOQ_MODULE_ENABLED: "true" | ||
| AWS_ACCESS_KEY_ID: ((aws_access_key_id)) | ||
| AWS_SECRET_ACCESS_KEY: ((aws_secret_access_key)) | ||
| AWS_DEFAULT_REGION: ((aws-region)) | ||
| CLOUDFRONT_DIST: ((cloudfront-dist)) | ||
| OUT_NAME: "nightly" | ||
| file: eloqdoc_src/concourse/tasks/build_nightly_tarball.yml |
There was a problem hiding this comment.
Fix task naming inconsistency: use build-eloqdss-eloqstore-type for ubuntu2404 to match ubuntu2204.
The ubuntu2404 eloqstore task is named eloqdss-eloqstore (line 209), while the ubuntu2204 equivalent is named build-eloqdss-eloqstore-type (line 130). For consistency and maintainability, use the same task name across both OS variants, or document why they differ.
Apply this diff to standardize the naming:
- - task: eloqdss-eloqstore
+ - task: build-eloqdss-eloqstore-type
image: image_ubuntu2404
params:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - task: eloqdss-eloqstore | |
| image: image_ubuntu2404 | |
| params: | |
| DATA_STORE_TYPE: "ELOQDSS_ELOQSTORE" | |
| BUILD_LOG_SRV: "true" | |
| GIT_SSH_KEY: ((git-key)) | |
| ASAN: "OFF" | |
| TAGGED: "" | |
| BUILD_TYPE: RelWithDebInfo | |
| ELOQ_MODULE_ENABLED: "true" | |
| AWS_ACCESS_KEY_ID: ((aws_access_key_id)) | |
| AWS_SECRET_ACCESS_KEY: ((aws_secret_access_key)) | |
| AWS_DEFAULT_REGION: ((aws-region)) | |
| CLOUDFRONT_DIST: ((cloudfront-dist)) | |
| OUT_NAME: "nightly" | |
| file: eloqdoc_src/concourse/tasks/build_nightly_tarball.yml | |
| - task: build-eloqdss-eloqstore-type | |
| image: image_ubuntu2404 | |
| params: | |
| DATA_STORE_TYPE: "ELOQDSS_ELOQSTORE" | |
| BUILD_LOG_SRV: "true" | |
| GIT_SSH_KEY: ((git-key)) | |
| ASAN: "OFF" | |
| TAGGED: "" | |
| BUILD_TYPE: RelWithDebInfo | |
| ELOQ_MODULE_ENABLED: "true" | |
| AWS_ACCESS_KEY_ID: ((aws_access_key_id)) | |
| AWS_SECRET_ACCESS_KEY: ((aws_secret_access_key)) | |
| AWS_DEFAULT_REGION: ((aws-region)) | |
| CLOUDFRONT_DIST: ((cloudfront-dist)) | |
| OUT_NAME: "nightly" | |
| file: eloqdoc_src/concourse/tasks/build_nightly_tarball.yml |
🤖 Prompt for AI Agents
In concourse/pipeline/build_nightly_tarball.yml around lines 209 to 224 the
ubuntu2404 task is named "eloqdss-eloqstore" while the ubuntu2204 equivalent
(line ~130) uses "build-eloqdss-eloqstore-type"; rename the ubuntu2404 task to
"build-eloqdss-eloqstore-type" to match, update any references or dependencies
elsewhere in the pipeline that point to the old name, and ensure the task file
path and params remain unchanged; alternatively, if the names must differ, add a
brief comment explaining the intentional difference.
Summary by CodeRabbit
New Features
Chores
Configuration
✏️ Tip: You can customize this high-level summary in your review settings.