add newa search subcommand#374
Merged
Merged
Conversation
Reviewer's GuideIntroduces a new Sequence diagram for the new search subcommandsequenceDiagram
actor User
participant main as main
participant cmd_search as cmd_search
participant ctx as CLIContext
participant _search_object as _search_object
participant print_state_dirs as print_state_dirs
User->>main: newa search [options]
main->>cmd_search: cmd_search(ctx, text, event, erratum, rog_mr, jira, brief, full)
cmd_search->>ctx: enter_command
cmd_search->>cmd_search: init_colors
cmd_search->>cmd_search: compile regex patterns
loop each state_dir in newa_statedir_topdir
cmd_search->>ctx: state_dirpath = state_dir
cmd_search->>cmd_search: evaluate search_specs
alt search_type == text
cmd_search->>cmd_search: state_dir.glob('*.yaml')
cmd_search->>cmd_search: value_pattern.search(content)
else search_type == jira
cmd_search->>ctx: load_jira_jobs
cmd_search->>_search_object: _search_object(jira_dict, key_pattern, value_pattern)
else search_type in [event, erratum, rog]
cmd_search->>ctx: load_artifact_jobs
cmd_search->>_search_object: _search_object(obj_dict, key_pattern, value_pattern)
end
alt all specs matched
cmd_search->>cmd_search: matching_state_dirs.append(state_dir)
end
end
cmd_search->>cmd_search: determine show_brief / show_events / show_issues
cmd_search->>print_state_dirs: print_state_dirs(ctx, matching_state_dirs, events, issues, False, False, False, brief)
print_state_dirs->>ctx: load_artifact_jobs(filter_events=True)
print_state_dirs->>print_state_dirs: _get_relative_time(state_dir)
print_state_dirs->>print_state_dirs: print headers and details
cmd_search-->>User: print summary of matches
Flow diagram for search output detail level selectionflowchart TD
A[Start cmd_search] --> B{brief flag?}
B -->|yes| C[show_brief = True]
C --> D[show_events = False\nshow_issues = False]
B -->|no| E{full flag?}
E -->|yes| F[show_events = False\nshow_issues = False]
E -->|no| G{jira option provided?}
G -->|yes| H[show_issues = True\nshow_events = False]
G -->|no| I[show_events = True\nshow_issues = False]
D --> J[call print_state_dirs]
F --> J
H --> J
I --> J
J --> K[End cmd_search]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The
--refresh/--refresh-allflags are still accepted and validated incmd_list, but they are never used insideprint_state_dirs(or elsewhere) to actually update Testing Farm request statuses anymore; consider wiring these options back to_update_all_tf_request_statuses(or equivalent) so the documented behavior is preserved. - In
cmd_search, the internal option name'rog'is used when building error messages for the--rog-mroption (e.g.,Invalid regular expression in --rog), which will be confusing to users; align the option label in error messages with the public flag name (e.g.,--rog-mr).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `--refresh`/`--refresh-all` flags are still accepted and validated in `cmd_list`, but they are never used inside `print_state_dirs` (or elsewhere) to actually update Testing Farm request statuses anymore; consider wiring these options back to `_update_all_tf_request_statuses` (or equivalent) so the documented behavior is preserved.
- In `cmd_search`, the internal option name `'rog'` is used when building error messages for the `--rog-mr` option (e.g., `Invalid regular expression in --rog`), which will be confusing to users; align the option label in error messages with the public flag name (e.g., `--rog-mr`).
## Individual Comments
### Comment 1
<location path="newa/cli/commands/list_cmd.py" line_range="352-360" />
<code_context>
+ else:
+ state_dirs = [Path(e.path) for e in sorted_entries[-last:]]
+
+ # Print all state directories
+ print_state_dirs(
+ ctx,
+ state_dirs,
+ events=events,
+ issues=issues,
+ refresh=refresh,
+ refresh_all=refresh_all,
+ specific_state_dir=specific_state_dir)
+
# restore logger level and statedir
</code_context>
<issue_to_address>
**issue (bug_risk):** The --refresh / --refresh-all flags no longer trigger any TF status updates
These flags are still accepted and passed through, but nothing in `cmd_list` / `print_state_dirs` calls `_update_all_tf_request_statuses` based on them, so they currently have no effect. If this behavior change isn’t deliberate, please reintroduce the refresh logic (e.g., call `_update_all_tf_request_statuses` before printing when `refresh` / `refresh_all` is set). If it is deliberate, consider removing or repurposing the flags to avoid confusing users.
</issue_to_address>
### Comment 2
<location path="newa/cli/commands/list_cmd.py" line_range="27-36" />
<code_context>
+def _get_relative_time(state_dir: Path) -> str:
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Broad exception handling in _get_relative_time can hide underlying problems
Catching `Exception` here means issues like permission errors or bad mtimes are silently converted to `''`, which can mask real problems. Please narrow the exception types (e.g. `OSError`, `ValueError`) or at least log the exception before returning `''`. Also, you call `datetime.now().astimezone().tzinfo` twice; store it once (e.g. `tz = datetime.now().astimezone().tzinfo`) and reuse it to avoid minor inconsistencies if the time changes between calls.
Suggested implementation:
```python
def _get_relative_time(state_dir: Path) -> str:
"""Get relative time string for state directory.
Uses the modification time of the .ppid file in the state directory.
Args:
state_dir: Path to the state directory
Returns:
Relative time string like "2 days ago", "3 hours ago", etc.
Returns empty string if ppid file not found.
```
```python
ppid_file = state_dir / ".ppid"
if not ppid_file.exists():
return ""
try:
tz = datetime.now().astimezone().tzinfo
mtime = ppid_file.stat().st_mtime
mtime_dt = datetime.fromtimestamp(mtime, tz=tz)
now = datetime.now(tz)
delta = now - mtime_dt
return humanize.naturaltime(delta)
except (OSError, ValueError):
# Failed to read or interpret the ppid file; treat as no relative time.
return ""
```
If this project has a logging convention (e.g. a module-level `logger`), you may optionally:
1. Import / obtain the logger in this module.
2. Log the caught exception inside the `except (OSError, ValueError):` block before returning `""`, for example: `logger.warning("Failed to compute relative time for %s", ppid_file, exc_info=True)`.
</issue_to_address>
### Comment 3
<location path="README.md" line_range="1107" />
<code_context>
+#### Option `--prev-state-dir`, `-P`
-Similar to `--state-dir`, however no directory is specified. Instead, `newa` will use the most recent (modified) directory used by `newa` process issued from the current shell (so the functionality won't collidate with `newa` processes from different terminals).
+Similar to `--state-dir`, however no directory is specified. Instead, `newa` will use the most recent (modified) directory used by `newa` process issued from the current shell.
+
+**Multi-Terminal Support:** This option is intentionally shell-scoped to support running multiple NEWA sessions in parallel across different terminals. Each terminal maintains its own "previous state-dir" context, allowing you to run different test campaigns simultaneously without interference:
</code_context>
<issue_to_address>
**nitpick (typo):** Clarify grammar around "newa process issued from the current shell"
Consider rephrasing to: "Instead, `newa` will use the most recently modified directory used by `newa` processes invoked from the current shell." This pluralizes "process" and replaces "issued from" with a more natural verb like "invoked from" or "run from."
```suggestion
Similar to `--state-dir`, however no directory is specified. Instead, `newa` will use the most recently modified directory used by `newa` processes invoked from the current shell.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
510b96b to
79eee1b
Compare
Implement 'newa search --text' command to search across all metadata files in state directories. The search supports regular expressions and is case-insensitive by default. Features: - Search all YAML files in all state directories - Case-insensitive regex pattern matching - Display event-level details for matching directories - Show state directory descriptions when available - Error handling for invalid regex patterns Implementation details: - Refactor list command to extract print_state_dirs() helper function - Both list and search commands share the same printing logic - Add comprehensive test coverage (13 tests total) Examples: newa search --text keylime newa search --text "RHEL-(9|10)\." newa search --text "SECENGSP-\d+" Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add --event, --erratum, --rog-mr, and --jira options to search within specific NEWA objects rather than across all metadata. New options: - --event PATTERN: Search within event objects - --erratum PATTERN: Search within erratum/advisory metadata - --rog-mr PATTERN: Search within RoG merge request objects - --jira PATTERN: Search within NEWA Jira tracker objects Search format supports two modes: - PATTERN: Search for pattern in any field of the object - KEY=PATTERN: Search only in fields where key matches KEY Key distinction: - --jira SECENGSP-10172 finds NEWA testing trackers - --erratum SECENGSP-10172 finds advisories fixing that Jira issue Multiple options use AND logic (all must match). Implementation: - Uses existing ctx.load_artifact_jobs() and ctx.load_jira_jobs() - Converts objects to dicts using attrs.asdict() - Implements recursive _search_object() helper function - Searches through nested dicts and lists Examples: newa search --erratum keylime newa search --jira "action_id=tier1" newa search --erratum "id=154960" --jira "SECENGSP-10172" Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit adds two main features:
1. **Relative time display**: State directory headers now show modification
time as relative time (e.g., "modified 2 hours ago", "modified 6 days ago")
using the most recently modified .ppid file in each state directory.
Format examples:
- /path/to/state-dir (modified 2 hours ago):
- /path/to/state-dir (description, modified 6 days ago):
The _get_relative_time() function calculates human-friendly time deltas
from "just now" up to months ago with proper singular/plural handling.
2. **Search output detail levels**: The search command now supports three
output detail levels via --brief and --full options:
- --brief: Only state directory headers (no events/issues/executions)
- Default: Intelligent behavior based on search criteria
* --event/--erratum/--rog-mr/--text: Show event-level details
* --jira: Show event-level + Jira issue details
- --full: Complete details including events, issues, and executions
(like default 'newa list' output)
The print_state_dirs() function in list_cmd.py now accepts a brief
parameter to support header-only output, avoiding code duplication.
Updated README.md with comprehensive documentation of the new output
detail levels and examples.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Python 3.11 doesn't support line breaks inside f-string braces like Python
3.13 does. Rewrote the multi-line f-strings using string concatenation to
maintain compatibility with Python 3.11+.
Changed from:
header = f'{
colorize_text(str(state_dir), state_dir_color)} ({description})'
To:
header = (f'{colorize_text(str(state_dir), state_dir_color)} '
f'({description})')
This fixes the SyntaxError: unterminated string literal errors on Python 3.11.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1. Fix error message for --rog-mr option: Changed internal 'rog' label to
'rog-mr' in error messages to match the public flag name that users see.
2. Improve _get_relative_time exception handling:
- Cache timezone (datetime.now().astimezone().tzinfo) to avoid potential
time changes between calls
- Narrow exception handling from broad 'Exception' to specific
'(OSError, ValueError)' to catch file access and time parsing errors
without hiding other problems
3. Fix README grammar: Changed "newa process issued from" to "newa
processes invoked from" for better clarity and grammatical correctness.
Note: The --refresh/--refresh-all flags are still functional. The refresh
logic exists at lines 217-225 in list_cmd.py where _update_all_tf_request_statuses
is called when these flags are set.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
79eee1b to
53427e5
Compare
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
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.
Summary by Sourcery
Add a searchable CLI for NEWA state directories and reuse listing logic for richer search output.
New Features:
searchsubcommand to query NEWA state directories by text, event, erratum, RoG MR, or Jira metadata with regex support and configurable output detail levels.Enhancements:
--prev-state-diroption, including multi-terminal behavior and examples.Tests:
searchcommand covering matching behavior, regex handling, output formatting, and error cases.