Add --rp-launch-uuid option to reuse existing ReportPortal launches#345
Merged
Conversation
Reviewer's GuideAdds a new --rp-launch-uuid option to the schedule command to reuse an existing ReportPortal launch, validates its usage against --no-reportportal, fetches launch metadata once, and applies it to all generated schedule jobs, alongside updated documentation. Sequence diagram for scheduling with --rp-launch-uuidsequenceDiagram
actor User
participant CLI as newa_CLI
participant ScheduleCmd as cmd_schedule
participant Helper as _process_jira_job
participant RPInit as initialize_rp_connection
participant RPClient as ReportPortalClient
participant RPServer as ReportPortal
User->>CLI: run schedule --rp-launch-uuid LAUNCH_UUID
CLI->>ScheduleCmd: invoke cmd_schedule(arch, fixtures, no_reportportal, rp_launch_uuid)
ScheduleCmd->>ScheduleCmd: validate no_reportportal and rp_launch_uuid are not both set
ScheduleCmd->>ScheduleCmd: initialize_state_dir(ctx)
ScheduleCmd->>ScheduleCmd: jira_jobs = _get_jira_jobs(ctx)
loop for each jira_job
ScheduleCmd->>Helper: _process_jira_job(ctx, jira_job, arch, fixtures, no_reportportal, rp_launch_uuid)
alt rp_launch_uuid is provided and first jira_job
Helper->>RPInit: initialize_rp_connection(ctx)
RPInit-->>Helper: rp_client
Helper->>RPClient: get_launch_info(rp_launch_uuid)
RPClient->>RPServer: HTTP GET launch metadata
RPServer-->>RPClient: launch_info
alt launch_info not found
Helper->>Helper: log error and exit(1)
else launch_info found
Helper->>RPClient: get_launch_url(rp_launch_uuid)
RPClient->>RPServer: build launch URL
RPServer-->>RPClient: launch_url
Helper->>Helper: cache launch_metadata (uuid, url, name, description)
end
end
Helper->>Helper: determine compose and architectures
Helper->>Helper: create RawRecipeSchedulerRequest instances
loop for each request in jira_job
Helper->>Helper: prepare jinja_vars and render request attributes
alt launch_metadata exists
Helper->>Helper: ensure request.reportportal exists
Helper->>Helper: apply launch_uuid, launch_url, launch_name, launch_description
end
Helper->>Helper: create ScheduleJob from request
Helper->>Helper: save ScheduleJob
end
end
File-Level Changes
Possibly linked issues
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 2 issues, and left some high level feedback:
- The
rp_launch_uuidparameter tocmd_scheduleis annotated asstrbut can beNone( Click default and mutual exclusivity check), so consider updating the type toOptional[str]to better reflect actual usage and avoid type-checker confusion. - Instead of calling
sys.exit(1)from within_process_jira_jobandcmd_schedule, consider raising aclick.ClickException(or another higher-level error) so that error handling and cleanup stay centralized in the CLI layer. - The mutual exclusivity between
--no-reportportaland--rp-launch-uuidis implemented manually; you might want to use Click's option group or custom callback mechanism to enforce this at argument parsing time and provide more standard error messages.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `rp_launch_uuid` parameter to `cmd_schedule` is annotated as `str` but can be `None` ( Click default and mutual exclusivity check), so consider updating the type to `Optional[str]` to better reflect actual usage and avoid type-checker confusion.
- Instead of calling `sys.exit(1)` from within `_process_jira_job` and `cmd_schedule`, consider raising a `click.ClickException` (or another higher-level error) so that error handling and cleanup stay centralized in the CLI layer.
- The mutual exclusivity between `--no-reportportal` and `--rp-launch-uuid` is implemented manually; you might want to use Click's option group or custom callback mechanism to enforce this at argument parsing time and provide more standard error messages.
## Individual Comments
### Comment 1
<location path="newa/cli/schedule_helpers.py" line_range="195-197" />
<code_context>
+ ctx.logger.info(f'Fetching ReportPortal launch {rp_launch_uuid}')
+ launch_info = rp.get_launch_info(rp_launch_uuid)
+
+ if not launch_info:
+ ctx.logger.error(
+ f'ERROR: Could not find ReportPortal launch {rp_launch_uuid} in project {
+ rp.project}')
+ sys.exit(1)
</code_context>
<issue_to_address>
**issue (bug_risk):** The multi-line f-string for the error log is syntactically invalid and will raise at import time.
The f-string is broken across lines inside the `{}` expression, which is invalid syntax and will raise a `SyntaxError`. Please keep the f-string on one line or join strings explicitly, for example:
```python
ctx.logger.error(
f'ERROR: Could not find ReportPortal launch {rp_launch_uuid} in project {rp.project}'
)
```
or:
```python
ctx.logger.error(
'ERROR: Could not find ReportPortal launch %s in project %s',
rp_launch_uuid,
rp.project,
)
```
</issue_to_address>
### Comment 2
<location path="newa/cli/commands/schedule_cmd.py" line_range="40-45" />
<code_context>
+ help='Reuse an existing ReportPortal launch UUID instead of creating a new one.',
+ )
@click.pass_obj
def cmd_schedule(
ctx: CLIContext,
arch: list[str],
fixtures: list[str],
- no_reportportal: bool) -> None:
+ no_reportportal: bool,
+ rp_launch_uuid: str) -> None:
"""
Schedule subcommand - creates schedule jobs from jira jobs.
</code_context>
<issue_to_address>
**suggestion:** The type annotation for `rp_launch_uuid` is too strict compared to the Click option configuration.
Because the Click option uses `default=None`, `rp_launch_uuid` is actually `Optional[str]` at runtime, but the signature declares `str`. This mismatch can mislead readers and type checkers. Consider updating the parameter to:
```python
rp_launch_uuid: Optional[str] = None
```
and relying on the function default instead of `default=None` in the decorator.
Suggested implementation:
```python
@click.option(
'--rp-launch-uuid',
help='Reuse an existing ReportPortal launch UUID instead of creating a new one.',
)
@click.pass_obj
def cmd_schedule(
ctx: CLIContext,
arch: list[str],
fixtures: list[str],
no_reportportal: bool,
rp_launch_uuid: Optional[str] = None) -> None:
```
1. Ensure `Optional` is imported from `typing` at the top of `newa/cli/commands/schedule_cmd.py`, e.g.:
- If there is an existing import: `from typing import Any, List`, extend it to `from typing import Any, List, Optional`.
- Otherwise, add a new line: `from typing import Optional`.
2. No other code changes are required, since the existing checks (`if no_reportportal and rp_launch_uuid:`) already correctly handle `None` vs. non-`None` values.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Introduces a new --rp-launch-uuid option for the schedule command that allows reusing an existing ReportPortal launch instead of creating a new one. This enables consolidating test results from multiple NEWA runs into a single ReportPortal launch. Key changes: - Add --rp-launch-uuid CLI option to schedule command (newa/cli/commands/schedule_cmd.py:34) - Implement mutual exclusivity validation between --rp-launch-uuid and --no-reportportal - Fetch and validate launch metadata from ReportPortal before scheduling jobs - Apply cached launch metadata (UUID, URL, name, description) to all generated schedule jobs - Update documentation with usage examples and important notes The launch UUID is validated before scheduling begins, and all generated requests inherit the launch metadata to ensure consistent reporting to the existing launch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
kkaarreell
force-pushed
the
ks_shedule_rp_uuid
branch
from
8a3f9b7 to
98df0dd
Compare
Collaborator
Author
|
/packit build |
The --fixture option now correctly handles values containing '=' characters by splitting only on the first '=' instead of rejecting any value with multiple equals signs. This fixes commands like: --fixture 'testingfarm.cli_args="--environment FOO=bar"' 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
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
1 participant
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 --rp-launch-uuid option for the schedule command that allows reusing an existing ReportPortal launch instead of creating a new one. This enables consolidating test results from multiple NEWA runs into a single ReportPortal launch.
Key changes:
The launch UUID is validated before scheduling begins, and all generated requests inherit the launch metadata to ensure consistent reporting to the existing launch.
🤖 Generated with Claude Code
Summary by Sourcery
Add support for reusing an existing ReportPortal launch when scheduling jobs and document the new workflow.
New Features:
Enhancements:
Documentation: