Skip to content

build: remove offloading form quickstart#129

Merged
forshev merged 1 commit into
mainfrom
0-qickstart-remove-offloading
Sep 11, 2025
Merged

build: remove offloading form quickstart#129
forshev merged 1 commit into
mainfrom
0-qickstart-remove-offloading

Conversation

@forshev
Copy link
Copy Markdown
Collaborator

@forshev forshev commented Sep 11, 2025

Description

remove offloading form quickstart


  • I have read and followed all requirements in CONTRIBUTING.md;
  • I used LLM/AI assistance to make this pull request;

If you have used LLM/AI assistance please provide model name and full prompt:

Model: {{model-name}}
Prompt: {{prompt}}

Summary by CodeRabbit

  • Chores

    • Removed bundled MinIO service from quickstart stacks; local setups no longer include object storage.
    • Removed offloading/remote-storage configuration from the quickstart config.
  • Refactor

    • Simplified slow-log settings to a single bulk threshold set to 1s.
  • Chores

    • Reduced default search limits to 100 and removed several client retry/keepalive and export/aggregation configuration options.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 11, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.49%. Comparing base (3a5ab3e) to head (86e3161).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #129      +/-   ##
==========================================
- Coverage   71.55%   71.49%   -0.07%     
==========================================
  Files         201      201              
  Lines       18170    18170              
==========================================
- Hits        13002    12991      -11     
- Misses       4451     4459       +8     
- Partials      717      720       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Sep 11, 2025

📝 Walkthrough

Walkthrough

Removed MinIO/offloading configuration from quickstart files, removed several seq-ui client and seq_api handler settings, and simplified slow_logs to a single bulk_threshold: 1s. Docker Compose MinIO service entries were deleted. Mapping and other sections left unchanged.

Changes

Cohort / File(s) Summary
Quickstart offloading & slow_logs
quickstart/config.quickstart.yaml
Removed entire offloading block. Replaced slow_logs search_threshold/fetch_threshold/previous bulk_threshold with a single bulk_threshold: 1s. mapping unchanged.
Seq-UI client & handler config
quickstart/config.seq-ui.yaml
Removed client keys (seq_db_avg_doc_size, request_retries, proxy_client_mode, grpc_keepalive_params) and many handlers.seq_api limits; added/updated handlers.seq_api.max_search_limit: 100.
Docker Compose — MinIO removal
quickstart/docker-compose.yaml, quickstart/docker-compose.seq-ui.yaml
Deleted minio service blocks (image minio/minio:latest, env MINIO_ROOT_USER/MINIO_ROOT_PASSWORD, ports 9000/9001, command). Other services unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • ssnd
  • moflotas

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly reflects the primary change (removing offloading from the quickstart, including MinIO removal) and follows a conventional "build:" prefix, so it accurately summarizes the main changes in the diff; however it contains a minor typo ("form" instead of "from") that slightly reduces clarity.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 0-qickstart-remove-offloading

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

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)
quickstart/config.quickstart.yaml (1)

9-13: Remove leftover offloading / MinIO / S3 references

Repo still contains many offloading/S3/MinIO artifacts — CI, tests, config, code, docs and dependencies must be removed or the offloading feature reinstated consistently.

  • tests/docker-compose.yml — MinIO service present (remove or update test infra).
  • .github/workflows/ci.yml — Setup MinIO step.
  • config.example.yaml — offloading defaults (endpoint, bucket, access_key, secret_key).
  • config/config.go + config/validation.go — Offloading config + validations.
  • cmd/seq-db/seq-db.go — initS3Client(cfg.Offloading...) and usage.
  • storage/s3/* (client.go, reader.go, uploader.go, reader_test.go) — S3 client implementation and tests.
  • frac/* (remote.go, sealed.go, active.go, processor) — Offload/Remote fraction logic.
  • fracmanager/* (fracmanager.go, fraction_provider.go, proxy_frac.go) — offloading orchestration and callers.
  • tests/setup/env.go + tests/integration_tests/offloading_test.go — tests create S3 buckets and rely on S3.
  • metric/store.go — Prometheus metrics with subsystem "offloading".
  • go.mod / go.sum — aws-sdk S3 deps still declared.
  • .github/CONTRIBUTING.md, docs, README snippets — references/labels mentioning offloading/MinIO.

Action: either remove all of the above (including tests/CI and go.mod deps) and update quickstart/config.quickstart.yaml, or keep offloading and ensure quickstart/docs/CI/tests reflect it.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3a5ab3e and 73a092b.

📒 Files selected for processing (3)
  • quickstart/config.quickstart.yaml (1 hunks)
  • quickstart/docker-compose.seq-ui.yaml (0 hunks)
  • quickstart/docker-compose.yaml (0 hunks)
💤 Files with no reviewable changes (2)
  • quickstart/docker-compose.yaml
  • quickstart/docker-compose.seq-ui.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: validate-pr-title
  • GitHub Check: test
🔇 Additional comments (1)
quickstart/config.quickstart.yaml (1)

9-12: No change required — use "storage.frac_size"
Key is correct and used consistently in code, validation, examples and docs (e.g. config/config.go, config/validation.go, config.example.yaml, quickstart/config.quickstart.yaml, docs/en/02-configuration.md).

Comment thread quickstart/config.quickstart.yaml
@forshev forshev force-pushed the 0-qickstart-remove-offloading branch from 73a092b to 86e3161 Compare September 11, 2025 10:16
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
quickstart/config.seq-ui.yaml (1)

11-11: Lowering max_search_limit to 100 — confirm intent and align consumers

If this reduces a previous default (e.g., 1000), verify UI pagination, API validators, and docs/changelog to avoid surprising users. Consider adding an inline comment for rationale.

Apply to add context:

   seq_api:
-    max_search_limit: 100
+    # Conservative limit in quickstart to keep demo queries lightweight; raise for prod use.
+    max_search_limit: 100
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 73a092b and 86e3161.

📒 Files selected for processing (4)
  • quickstart/config.quickstart.yaml (1 hunks)
  • quickstart/config.seq-ui.yaml (1 hunks)
  • quickstart/docker-compose.seq-ui.yaml (0 hunks)
  • quickstart/docker-compose.yaml (0 hunks)
💤 Files with no reviewable changes (2)
  • quickstart/docker-compose.seq-ui.yaml
  • quickstart/docker-compose.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
  • quickstart/config.quickstart.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: test

@forshev forshev merged commit 09d9579 into main Sep 11, 2025
9 checks passed
@forshev forshev deleted the 0-qickstart-remove-offloading branch September 11, 2025 10:31
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.

4 participants