Fix Jira Cloud compatibility in jira notificator#957
Fix Jira Cloud compatibility in jira notificator#957openshift-merge-bot[bot] merged 2 commits intoopenshift:mainfrom
Conversation
Replace deprecated search_issues (startAt pagination) with enhanced_search_issues (nextPageToken pagination) for Jira Cloud. Bump jira minimum version to 3.8.0 which introduced the new API. Update test fixture to use accessible issue OCPBUGS-78840.
|
/cc @tomasdavidorg |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughRemoved the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_jira_notificator.py (1)
341-356:⚠️ Potential issue | 🟡 MinorTest expectation is out of sync with production code.
The test expects Target Versions ending at
4.19.z, but the actualget_on_qa_filtermethod (line 624) includes4.20.zand4.21.z. This test will fail.🐛 Proposed fix
def test_get_on_qa_filter(self): self.assertEqual( self.ns.get_on_qa_filter(None), ( "project = OCPBUGS AND issuetype in (Bug, Vulnerability) " - "AND status = ON_QA AND 'Target Version' in (4.12.z, 4.13.z, 4.14.z, 4.15.z, 4.16.z, 4.17.z, 4.18.z, 4.19.z)" + "AND status = ON_QA AND 'Target Version' in (4.12.z, 4.13.z, 4.14.z, 4.15.z, 4.16.z, 4.17.z, 4.18.z, 4.19.z, 4.20.z, 4.21.z)" ) ) self.assertEqual( self.ns.get_on_qa_filter(datetime(2025, 7, 17, tzinfo=timezone.utc)), ( "project = OCPBUGS AND issuetype in (Bug, Vulnerability) " - "AND status = ON_QA AND 'Target Version' in (4.12.z, 4.13.z, 4.14.z, 4.15.z, 4.16.z, 4.17.z, 4.18.z, 4.19.z)" + "AND status = ON_QA AND 'Target Version' in (4.12.z, 4.13.z, 4.14.z, 4.15.z, 4.16.z, 4.17.z, 4.18.z, 4.19.z, 4.20.z, 4.21.z)" " AND status changed to ON_QA after 2025-07-17" ) )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_jira_notificator.py` around lines 341 - 356, The test expectation in test_get_on_qa_filter is stale: update the expected JQL strings used in self.assertEqual calls for ns.get_on_qa_filter (both the None case and the datetime case) to include the additional Target Versions 4.20.z and 4.21.z so they match the production get_on_qa_filter implementation; ensure the datetime branch still appends " AND status changed to ON_QA after YYYY-MM-DD" exactly as returned by get_on_qa_filter.
🧹 Nitpick comments (2)
oar/notificator/jira_notificator.py (2)
684-684: CLI option--search-batch-sizeis now ineffective.This option no longer controls pagination behavior since
enhanced_search_issueshandles it internally. Consider deprecating or removing this option to avoid user confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@oar/notificator/jira_notificator.py` at line 684, The CLI option --search-batch-size is now ineffective because pagination is handled inside enhanced_search_issues; remove or deprecate this option by deleting the click.option("--search-batch-size", ...) declaration (or replace it with a no-op deprecated flag) and any references to it so users are not misled. Update the command's help text and any documentation strings to reflect that enhanced_search_issues controls pagination internally and, if you choose deprecation instead of removal, log a warning in the CLI entrypoint (the function decorated with `@click.command` that consumes the option) indicating the flag is ignored and will be removed in a future release.
631-652:search_batch_sizeparameter is unused and docstring is misleading.The
search_batch_sizeparameter is accepted but no longer used in the method body sinceenhanced_search_issueshandles pagination internally withmaxResults=0. The docstring claiming it specifies "Number of issues to fetch per batch" is now inaccurate. Consider removing this parameter from the method signature and updating the docstring, along with callers inprocess_on_qa_issuesand the CLI option.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@oar/notificator/jira_notificator.py` around lines 631 - 652, The get_on_qa_issues method still accepts a now-unused parameter search_batch_size and contains an inaccurate docstring; remove the search_batch_size parameter from get_on_qa_issues, update its docstring to reflect that enhanced_search_issues handles pagination (no batch size), and update all callers (e.g., process_on_qa_issues) and any CLI option that passes a batch size so they no longer supply this argument; keep the method name get_on_qa_issues and ensure calls use the single from_date argument instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@tests/test_jira_notificator.py`:
- Around line 341-356: The test expectation in test_get_on_qa_filter is stale:
update the expected JQL strings used in self.assertEqual calls for
ns.get_on_qa_filter (both the None case and the datetime case) to include the
additional Target Versions 4.20.z and 4.21.z so they match the production
get_on_qa_filter implementation; ensure the datetime branch still appends " AND
status changed to ON_QA after YYYY-MM-DD" exactly as returned by
get_on_qa_filter.
---
Nitpick comments:
In `@oar/notificator/jira_notificator.py`:
- Line 684: The CLI option --search-batch-size is now ineffective because
pagination is handled inside enhanced_search_issues; remove or deprecate this
option by deleting the click.option("--search-batch-size", ...) declaration (or
replace it with a no-op deprecated flag) and any references to it so users are
not misled. Update the command's help text and any documentation strings to
reflect that enhanced_search_issues controls pagination internally and, if you
choose deprecation instead of removal, log a warning in the CLI entrypoint (the
function decorated with `@click.command` that consumes the option) indicating the
flag is ignored and will be removed in a future release.
- Around line 631-652: The get_on_qa_issues method still accepts a now-unused
parameter search_batch_size and contains an inaccurate docstring; remove the
search_batch_size parameter from get_on_qa_issues, update its docstring to
reflect that enhanced_search_issues handles pagination (no batch size), and
update all callers (e.g., process_on_qa_issues) and any CLI option that passes a
batch size so they no longer supply this argument; keep the method name
get_on_qa_issues and ensure calls use the single from_date argument instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3c4e9e45-83e8-4e3a-9b59-d2913fdbcc2e
📒 Files selected for processing (3)
oar/notificator/jira_notificator.pypyproject.tomltests/test_jira_notificator.py
oar/notificator/jira_notificator.py
Outdated
There was a problem hiding this comment.
search_batch_size is unused if we use maxResults=0
There was a problem hiding this comment.
yes, this is a problem, I'm investigating how to support this option with pagination pattern
There was a problem hiding this comment.
Good catch. This has been addressed in the latest commit - search_batch_size is now removed from get_on_qa_issues, process_on_qa_issues, and the CLI.
Here's the reasoning behind the design:
The Jira Cloud search API (/rest/api/2/search/jql) has a hard limit of 100 results per page regardless of the maxResults value you pass - see Jira Cloud REST API docs. This is a Jira Cloud vs Jira Server distinction, not a v2 vs v3 API distinction - the search/jql endpoint with nextPageToken pagination is a Cloud-specific extension available in both v2 and v3.
The enhanced_search_issues(maxResults=0) call instructs the jira library to automatically paginate through all pages (100 items per request) using nextPageToken until all results are fetched.
The old search_batch_size was controlling the startAt-based pagination page size on Jira Server - a concept that no longer applies on Jira Cloud where:
maxResults > 0→ fetches one page (capped at 100), then stopsmaxResults = 0→ fetches all pages automatically, 100 per request
So search_batch_size was effectively a no-op on Cloud for values > 100, and for values ≤ 100 it only limited results to a single partial page. Removing it is the correct approach.
Remove search_batch_size parameter from get_on_qa_issues and process_on_qa_issues since Jira Cloud pagination is handled internally by enhanced_search_issues. Update CLI to drop --search-batch-size option. Fix test_get_on_qa_filter to include 4.20.z and 4.21.z target versions.
|
@coderabbitai please double check latest commit and other review comments, is it ok to merge |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me check the current state of the PR, files, and any outstanding review comments. 🏁 Script executed: Length of output: 30811
✅ All code review issues are addressed
✅ CI: All tests passed⏳ Pending: Formal approvalMy second automated review on the latest commit ( |
|
@tomasdavidorg can you review again. openshift/release#76321 needs to be updated based latest change for cli |
|
@rioliu-rh: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tomasdavidorg The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
2 participants
Summary
Fixes jira notificator CI failure caused by deprecated Jira Cloud search API.
search_issues(uses deprecatedstartAtpagination) withenhanced_search_issues(usesnextPageTokenpagination) for Jira Cloud compatibilityjiralibrary version from3.4.1to3.8.0, which introducedenhanced_search_issuesOCPBUGS-1542(inaccessible to bot account) withOCPBUGS-78840(same characteristics: no assignee)Fixes: https://redhat.atlassian.net/browse/OCPERT-339