Open
Conversation
Reviewer's GuideThis PR adds optional configuration flags to include clickable Testing Farm and Jira URLs in newa CLI logs, wiring the settings through the config loader and updating logging in Jira helpers and Testing Farm worker output to show full URLs when enabled. Class diagram for updated Settings configuration optionsclassDiagram
class Settings {
# jira_token : str
# jira_email : str
# jira_project : str
# jira_show_url : bool
# tf_token : str
# tf_recheck_delay : str
# tf_show_url : bool
# rog_token : str
# ai_api_url : str
# ai_api_token : str
}
class ConfigParser {
}
class SettingsLoader {
+load_settings(config_parser cp) Settings
-_get(config_parser cp, str key, str env_var, str default_value) str
-_str_to_bool(str value) bool
}
ConfigParser <.. SettingsLoader : uses
SettingsLoader ..> Settings : creates
Flow diagram for TF worker logging with optional URLflowchart TD
A[tf_worker starts handling TF request] --> B[Read tf_request details]
B --> C{ctx.settings.tf_show_url?}
C -- true --> D[log TF request execute_job.execution.artifacts_url with envs and state]
C -- false --> E[log TF request tf_request.uuid with envs and state]
D --> F[Check tf_request.is_finished]
E --> F[Check tf_request.is_finished]
Flow diagram for Jira helper logging with optional URLsflowchart TD
A[_find_or_create_issue called] --> B[Iterate search_result items]
B --> C{ctx.settings.jira_show_url?}
C -- true --> D[logger.debug Checking key with ctx.settings.jira_url/browse/key]
C -- false --> E[logger.debug Checking key]
D --> F[Determine opened_issues and closed_issues]
E --> F[Determine opened_issues and closed_issues]
F --> G{opened_issues empty?}
G -- yes --> H{ctx.settings.jira_show_url?}
H -- true --> I[logger.info Relevant closed_ids with ctx.settings.jira_url/browse/closed_ids found but already closed]
H -- false --> J[logger.info Relevant closed_ids found but already closed]
G -- no --> K{Creating new issue?}
K -- yes --> L{ctx.settings.jira_show_url?}
L -- true --> M[logger.info New issue id created with ctx.settings.jira_url/browse/id]
L -- false --> N[logger.info New issue id created]
K -- no --> O{Reusing existing issue}
O --> P{ctx.settings.jira_show_url?}
P -- true --> Q[logger.info Issue new_issue with ctx.settings.jira_url/browse/new_issue re-used]
P -- false --> R[logger.info Issue new_issue re-used]
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 1 issue, and left some high level feedback:
- In the
Relevant issues {closed_ids}branch you build a single Jira URL usingclosed_idswhich can contain multiple comma-separated IDs, resulting in an invalid link; consider logging individual issue URLs or omitting the URL when multiple IDs are present. - There is repeated
if ctx.settings.jira_show_url/tf_show_urllogging logic that only differs in message formatting; consider extracting small helper functions to format Jira and TF log messages to keep this behavior consistent and easier to maintain. - In the
Issue {new_issue} re-usedlog withjira_show_urlenabled, the message concatenation lacks a space beforere-used(.../{new_issue})re-used), which makes the log a bit hard to read; adding a space or using a single f-string would fix this.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the `Relevant issues {closed_ids}` branch you build a single Jira URL using `closed_ids` which can contain multiple comma-separated IDs, resulting in an invalid link; consider logging individual issue URLs or omitting the URL when multiple IDs are present.
- There is repeated `if ctx.settings.jira_show_url` / `tf_show_url` logging logic that only differs in message formatting; consider extracting small helper functions to format Jira and TF log messages to keep this behavior consistent and easier to maintain.
- In the `Issue {new_issue} re-used` log with `jira_show_url` enabled, the message concatenation lacks a space before `re-used` (`.../{new_issue})re-used`), which makes the log a bit hard to read; adding a space or using a single f-string would fix this.
## Individual Comments
### Comment 1
<location path="newa/cli/jira_helpers.py" line_range="306-314" />
<code_context>
for jira_issue_key, jira_issue in search_result.items():
- ctx.logger.debug(f"Checking {jira_issue_key}")
+ if ctx.settings.jira_show_url:
+ ctx.logger.debug(
+ f"Checking {jira_issue_key} "
</code_context>
<issue_to_address>
**issue (bug_risk):** Building a single JIRA URL from a comma-separated list of IDs will produce an invalid link.
Since `closed_ids` is a comma-separated string, using it in a single `/browse/{closed_ids}` path will confuse users with a non-functional link. If you want clickable links, either emit one URL per ID or log the base JIRA URL separately and keep the IDs as a list/string.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+306
to
314
| if ctx.settings.jira_show_url: | ||
| ctx.logger.debug( | ||
| f"Checking {jira_issue_key} " | ||
| f"({ctx.settings.jira_url}/browse/{jira_issue_key})") | ||
| else: | ||
| ctx.logger.debug(f"Checking {jira_issue_key}") | ||
|
|
||
| is_new = False | ||
| if jira_handler.newa_id(action) in jira_issue["description"] \ |
There was a problem hiding this comment.
issue (bug_risk): Building a single JIRA URL from a comma-separated list of IDs will produce an invalid link.
Since closed_ids is a comma-separated string, using it in a single /browse/{closed_ids} path will confuse users with a non-functional link. If you want clickable links, either emit one URL per ID or log the base JIRA URL separately and keep the IDs as a list/string.
Collaborator
|
Hi @bajertom , what output are you typically consuming? |
Contributor
Author
|
The output that newa continuously sends to terminal, like this one: You're right about Feel free to close it as a Won't fix, I could debug in hatch development environment using this code change. |
Collaborator
I am OK adding the functionality, I would just prefer to have a single option (something like |
kkaarreell
reviewed
Mar 11, 2026
| log(f'TF request {tf_request.uuid} envs: {envs} state: {state}') | ||
| if ctx.settings.tf_show_url: | ||
| log( | ||
| f'TF request {execute_job.execution.artifacts_url}' |
Collaborator
There was a problem hiding this comment.
If a request hasn't been scheduled yet, there won't be any artifacts URL. See line 118.
This is in fact the reason we are logging only UUIDs because early after submitting we have only URL to TF request API and not artifacts URL.
kkaarreell
reviewed
Mar 11, 2026
| if ctx.settings.jira_show_url: | ||
| ctx.logger.info( | ||
| f"Relevant issues {closed_ids} " | ||
| f"({ctx.settings.jira_url}/browse/{closed_ids}) found but already closed") |
Collaborator
There was a problem hiding this comment.
closed_ids is a list, this won't create a functional link.
kkaarreell
changed the title
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
2 participants
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.
For debugging/configuring purposes I often run newa manually from the command line. I don't wait for the complete results, I just want to quickly get to the log on Testing Farm or see the Jira issues. That means highlighting the TF UUID/jira issue number in the newa output stream with a mouse, alt+shift+D to open
doerand then mouse clicking on the correct line to open the desired interface. (Note: that does not work for staging jira instance, because that is not even present in thedoerUI list)This change introduces two options in newa.conf file to allow printing clickable links to TF/Jira in the newa output stream and getting the results quickly without the overhead of mouse-selecting and sometimes missclicking in
doer.Summary by Sourcery
Add configurable options to include clickable Testing Farm and Jira URLs in CLI logs for easier navigation from terminal output.
New Features:
Enhancements: