Skip to content

feat(cli): support for default file/dir ignore config in ovcli.conf#1393

Merged
qin-ctx merged 1 commit intovolcengine:mainfrom
sentisso:main
Apr 13, 2026
Merged

feat(cli): support for default file/dir ignore config in ovcli.conf#1393
qin-ctx merged 1 commit intovolcengine:mainfrom
sentisso:main

Conversation

@sentisso
Copy link
Copy Markdown
Contributor

Description

Add default upload filter support in ovcli.conf for CLI add-resource, and merge those defaults with per-command CLI flags (--ignore-dirs, --include, --exclude) instead of overriding.
Also extend Python openviking_cli config schema so new upload section is accepted without validation failure.

Related Issue

Fixes #1386

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

  • Extended Rust CLI config schema (crates/ov_cli/src/config.rs) with upload.ignore_dirs, upload.include, upload.exclude.
  • Added merge logic for upload filters in add-resource flow (crates/ov_cli/src/main.rs): config defaults + CLI values merged additively.
  • Kept request payload contract unchanged (ignore_dirs/include/exclude still optional CSV strings sent to /api/v1/resources).
  • Extended Python OVCLIConfig (openviking_cli/utils/config/ovcli_config.py) with optional nested upload model for compatibility.
  • Added/updated tests:
    • Rust unit tests for config deserialization + merge behavior.
    • Python tests for accepting valid upload section and rejecting unknown nested upload fields.
  • Updated docs/examples:
    • crates/ov_cli/README.md
    • docs/en/guides/01-configuration.md
    • docs/zh/guides/01-configuration.md

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

Test commands/results:

  • cargo test -p ov_cli ✅ (15 passed)
  • pytest --noconftest tests/client/test_http_client_config.py -q ✅ (6 passed)
  • pytest tests/client/test_http_client_config.py ⚠️ environment hit SIGSEGV via global test bootstrap in this setup

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Screenshots (if applicable)

N/A (CLI/config/docs changes only).

Additional Notes

  • Per issue request, .gitignore support intentionally not implemented in this PR.
  • openviking_cli compatibility update included because OVCLIConfig uses strict extra="forbid"; without this, adding upload to ovcli.conf would break Python config loading.
  • cargo fmt applied; includes minor format-only changes.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 12, 2026

CLA assistant check
All committers have signed the CLA.

@sentisso sentisso changed the title feat: dir ignore support in ovcli.conf feat: default file/dir ignore support in ovcli.conf Apr 12, 2026
@sentisso sentisso changed the title feat: default file/dir ignore support in ovcli.conf feat(cli): default file/dir ignore support in ovcli.conf Apr 12, 2026
@github-actions
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1386 - Partially compliant

Compliant requirements:

  • Support configurable ignore rules in ovcli.conf for CLI uploads
  • Merge ignore rules from config and CLI flags additively
  • Extend ovcli.conf with upload.ignore_dirs, upload.include, upload.exclude

Non-compliant requirements:

  • Respect .gitignore files by default (optional)
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🏅 Score: 90
🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

@github-actions
Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Reject unknown upload config fields

Add #[serde(deny_unknown_fields)] to UploadConfig to reject unknown fields in the
upload section, matching the Python config's strict validation behavior. This
prevents silent ignoring of typos or invalid keys.

crates/ov_cli/src/config.rs [8-13]

 #[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(deny_unknown_fields)]
 pub struct UploadConfig {
     pub ignore_dirs: Option<String>,
     pub include: Option<String>,
     pub exclude: Option<String>,
 }
Suggestion importance[1-10]: 7

__

Why: This aligns the Rust config validation with the Python side's extra: "forbid" behavior, preventing silent ignoring of typos or invalid keys in the upload section, which improves correctness and consistency.

Medium

@sentisso sentisso changed the title feat(cli): default file/dir ignore support in ovcli.conf feat(cli): support for default file/dir ignore config in ovcli.conf Apr 12, 2026
@qin-ctx qin-ctx merged commit 7388e4d into volcengine:main Apr 13, 2026
2 checks passed
@github-project-automation github-project-automation bot moved this from Backlog to Done in OpenViking project Apr 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Feature]: Configurable file/dir ignores for CLI uploads: support ovcli.conf and .gitignore defaults

3 participants