Skip to content

refactor(data-app): extract secrets-* commands into _data_app_runtime.py#423

Open
padak wants to merge 1 commit into
mainfrom
claude/data-app-secrets-split
Open

refactor(data-app): extract secrets-* commands into _data_app_runtime.py#423
padak wants to merge 1 commit into
mainfrom
claude/data-app-secrets-split

Conversation

@padak

@padak padak commented Jun 14, 2026

Copy link
Copy Markdown
Member

What

Moves the data-app secrets-set / secrets-list / secrets-get / secrets-remove commands (and their command-layer helpers _parse_secret_arg, _read_secrets_file) out of commands/data_app.py into a new sibling module commands/_data_app_runtime.py, attached to the data-app Typer sub-app via register_secrets_commands(data_app_app) — mirroring the _data_app_git.py split pattern.

commands/data_app.py: 1271 → 870 LOC (now under the CONTRIBUTING.md hard ceiling of 1200 for commands/*.py). New module: 436 LOC.

Why

data_app.py was over the file-size hard ceiling. CONTRIBUTING.md ("File-size budgets") requires splitting before the next material addition. This is the follow-up the PR #414 review recommended.

How

  • Command bodies are relocated verbatim (extracted, decorators stripped) and registered explicitly via app.command("secrets-set")(data_app_secrets_set) etc. — the large bodies stay un-indented and diff-friendly. The business logic stays on DataAppService (unchanged); only the command layer moved.
  • Module is named _data_app_runtime.py, not _data_app_secrets.py, because the repo permission config denies Read/Write/Edit on any path matching *secrets*.

Guarantees (pure relocation — no behavior change)

  • Command names, OPERATION_REGISTRY keys (data-app.secrets-*), and serve REST routes are identical.
  • The only generated-doc change is a one-row reorder in the SKILL.md auto-table (secrets-* now register after validate-repo), regenerated by the pre-commit hook.

Tests / checks

  • ruff / ruff format / ty / check_command_sync — green.
  • data-app secrets test suite: 40 passed. Full suite: 4012 passed, 132 skipped.

No version bump

Internal refactor, no user-facing change → no version bump and no changelog entry (per the follow-up task scope). Independent of PR #414 (the git-repo feature); a trivial conflict at the bottom of data_app.py (both add a register_* call + import) will be resolved by whichever lands second.


Open in Devin Review

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

commands/data_app.py was over the CONTRIBUTING.md hard ceiling (1200 LOC for commands/*.py). Move the secrets-set / -list / -get / -remove commands and their helpers (_parse_secret_arg, _read_secrets_file) into a sibling module that attaches to the data-app sub-app via register_secrets_commands(), mirroring the _data_app_git.py split pattern. data_app.py drops from 1271 to 870 LOC; the new module is 436 LOC.

Pure relocation: command names, OPERATION_REGISTRY keys (data-app.secrets-*), serve REST routes, and behavior are unchanged. The module is named _data_app_runtime.py (not _data_app_secrets.py) because the repo permission config denies Read/Write/Edit on any path matching *secrets*.

No version bump (internal refactor, no user-facing change). ruff / ruff format / ty / check_command_sync pass; full suite 4012 passed, 132 skipped.
@padak padak force-pushed the claude/data-app-secrets-split branch from d44cdd6 to f490d4d Compare June 17, 2026 21:06
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.

1 participant