Add -C/--copy-state-dir option to copy state from existing directory #305
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideAdds a new -C/--copy-state-dir CLI option to copy YAML files from an existing state directory into a newly initialized state directory, enforces mutual exclusivity with --extract-state-dir, and documents the new behavior in the README. Flow diagram for handling --extract-state-dir and --copy-state-dir optionsflowchart TD
Start[Start newa CLI] --> ParseOptions[Parse CLI options]
ParseOptions --> CheckMutualExcl[Check extract_state_dir and copy_state_dir]
CheckMutualExcl -->|both set| Error[Raise ClickException: Cannot use both options]
CheckMutualExcl -->|only extract_state_dir| DoExtract[Extract YAML from archive to state_dirpath]
CheckMutualExcl -->|only copy_state_dir| DoCopy[Copy YAML from source state dir to new state_dirpath]
CheckMutualExcl -->|neither set| Continue[Continue with existing or new state_dirpath]
DoExtract --> ForceFlag[Set ctx.force true]
ForceFlag --> OpenArchive[Open archive via tarfile]
OpenArchive --> ExtractMembers[Extract YAML files into ctx.state_dirpath]
ExtractMembers --> InitDirExtract[initialize_state_dir ctx]
InitDirExtract --> NextStepsExtract[Proceed with further CLI actions]
DoCopy --> ResolveSource[Resolve and validate source_dir path]
ResolveSource -->|not exists or not dir| ErrorSource[Raise ClickException for invalid source_dir]
ResolveSource --> InitDirCopy[initialize_state_dir ctx]
InitDirCopy --> FindYaml[List *.yaml files in source_dir]
FindYaml -->|none found| WarnNone[Log warning: no YAML files]
FindYaml -->|one or more| CopyLoop[Copy each YAML file to ctx.state_dirpath]
CopyLoop --> LogCopied[Log number of copied YAML files]
WarnNone --> NextStepsCopy[Proceed with further CLI actions]
LogCopied --> NextStepsCopy[Proceed with further CLI actions]
Continue --> NextStepsNoStateOp[Proceed with further CLI actions]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- The
initialize_state_dirhelper is now called from both the--extract-state-dirand--copy-state-dirbranches but the local import inside the extract branch was removed; ensure it is imported at module scope (or otherwise available) so both code paths work correctly. - The copy logic only includes
*.yamlfiles; if your state dirs may contain.ymlfiles or YAML in subdirectories, consider whether those should also be copied and adjust the globbing accordingly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `initialize_state_dir` helper is now called from both the `--extract-state-dir` and `--copy-state-dir` branches but the local import inside the extract branch was removed; ensure it is imported at module scope (or otherwise available) so both code paths work correctly.
- The copy logic only includes `*.yaml` files; if your state dirs may contain `.yml` files or YAML in subdirectories, consider whether those should also be copied and adjust the globbing accordingly.
## Individual Comments
### Comment 1
<location> `newa/cli/main.py:220` </location>
<code_context>
+ # copy YAML files from the given state directory to a new state-dir
+ if copy_state_dir:
+ source_dir = Path(os.path.expandvars(copy_state_dir))
+ if not source_dir.exists():
+ raise click.ClickException(
</code_context>
<issue_to_address>
**suggestion:** Consider also expanding `~` in paths, not just environment variables.
`os.path.expandvars` only handles `$VAR` placeholders, so values like `~/state-dir` won’t resolve. Consider wrapping with `os.path.expanduser` as well, e.g. `Path(os.path.expanduser(os.path.expandvars(copy_state_dir)))`, to support both env vars and home-directory shortcuts.
Suggested implementation:
```python
source_dir = Path(os.path.expanduser(os.path.expandvars(copy_state_dir)))
```
If `os` is not already imported at the top of `newa/cli/main.py`, add:
```python
import os
```
near the other imports so that `os.path.expanduser` and `os.path.expandvars` are available.
</issue_to_address>
### Comment 2
<location> `README.md:727` </location>
<code_context>
-#### Option `--extract-state-dir`
+#### Option `--extract-state-dir`, `-E`
Similar to `--state-dir`, however in this case the argument is URL of an archive containing NEWA YAML metadata files. For example, it could be used to follow up on a state-dir created and shared by an automation.
</code_context>
<issue_to_address>
**suggestion (typo):** Consider adding "the" before "URL" for smoother grammar.
Please update the sentence to: "the argument is the URL of an archive containing NEWA YAML metadata files."
```suggestion
Similar to `--state-dir`, however in this case the argument is the URL of an archive containing NEWA YAML metadata files. For example, it could be used to follow up on a state-dir created and shared by an automation.
```
</issue_to_address>
### Comment 3
<location> `README.md:750-752` </location>
<code_context>
+$ newa --copy-state-dir /mnt/shared/newa-state-dir/run-456 jira --issue-config config.yaml schedule execute report
+```
+
#### Option `--context, -c`
Allows custom `tmt` context definition on a cmdline. Such a context can be used in issue-config YAML file through Jinja template through `CONTEXT.<name>`. Option can be used multiple times.
</code_context>
<issue_to_address>
**suggestion (typo):** Wording "on a cmdline" is a bit awkward; consider "on the command line".
This small change will improve readability of the documentation for users.
</issue_to_address>
### Comment 4
<location> `README.md:752` </location>
<code_context>
+
#### Option `--context, -c`
Allows custom `tmt` context definition on a cmdline. Such a context can be used in issue-config YAML file through Jinja template through `CONTEXT.<name>`. Option can be used multiple times.
</code_context>
<issue_to_address>
**issue (typo):** Duplicated "through" in this sentence looks like a typo.
The phrase "through Jinja template through `CONTEXT.<name>`" repeats "through". Consider changing it to something like "in an issue-config YAML file via a Jinja template using `CONTEXT.<name>`" or otherwise removing the duplicate word.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
kkaarreell
force-pushed
the
ks_cli_copy
branch
from
7e6dc7f to
5ebb393
Compare
Introduces a new CLI option -C/--copy-state-dir that allows users to copy YAML files from an existing state directory to a newly created state-dir. This is useful for reusing state from a different machine or shared location without modifying the original state-dir. Key features: - Copies all YAML files from the specified source directory - Creates a new state directory with proper initialization - Validates source directory existence before copying - Cannot be used together with --extract-state-dir (mutual exclusivity check) - Logs copying progress with info and debug messages 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
kkaarreell
force-pushed
the
ks_cli_copy
branch
5 times, most recently
from
18689a1 to
888d4a0
Compare
kkaarreell
force-pushed
the
ks_cli_copy
branch
from
888d4a0 to
9e3c9a6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduces a new CLI option -C/--copy-state-dir that allows users to copy YAML files from an existing state directory to a newly created state-dir. This is useful for reusing state from a different machine or shared location without modifying the original state-dir.
Key features:
🤖 Generated with Claude Code
Summary by Sourcery
Add support for initializing a new state directory by copying YAML files from an existing state directory, with safeguards around usage.
New Features:
Enhancements:
Documentation: