feat: add Jira Cloud API support#392
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new UAT test script for the Jira Cloud API, which covers various functionalities like issue creation, commenting, labeling, status changes, attachments, and JQL searches, including a new test for Jotnar tags. The jira_utils.py file was updated to better support both Jira Cloud and Server APIs, introducing an is_jira_cloud helper and modifying API calls (jira_api_get, jira_api_post, jira_api_put) to allow specifying the API version (v2 or v3) for better compatibility. However, several critical issues were identified: the URL construction logic in jira_api_get, jira_api_post, and jira_api_put was simplified in a way that breaks existing callers providing full REST paths. The get_current_issues function has a critical bug where the jira_api_post call was removed, leading to an UnboundLocalError, and its pagination logic is broken. The get_user_account_id function, when used with Jira Server, will likely fail because it defaults to API v3, which is only available on Jira Cloud. Additionally, there's a minor inconsistency in the UAT test script's progress messages, showing x/14 instead of x/15 in some places.
| def jira_api_get(path: str, *, params: dict | None = None) -> Any: | ||
| url = f"{jira_url()}/{path}" if path.startswith("rest/") else f"{jira_url()}/rest/api/2/{path}" | ||
| def jira_api_get(path: str, *, params: dict | None = None, api_version: Literal["2", "3"] = "3") -> Any: | ||
| url = f"{jira_url()}/rest/api/{api_version}/{path}" |
There was a problem hiding this comment.
The new URL construction logic breaks callers that provide a full REST path. For example, JIRA_SEARCH_PATH is rest/api/3/search/jql, and calling this function with it will result in a malformed URL like .../rest/api/3/rest/api/3/search/jql. You should retain the logic to handle paths that already start with rest/.
This issue also exists in jira_api_post, jira_api_put, and jira_api_upload.
| url = f"{jira_url()}/rest/api/{api_version}/{path}" | |
| url = f"{jira_url()}/{path}" if path.startswith("rest/") else f"{jira_url()}/rest/api/{api_version}/{path}" |
supervisor/jira_utils.py
Outdated
| body: dict[str, Any] = { | ||
| "jql": jql, | ||
| "maxResults": max_results, | ||
| "fields": _fields(full), | ||
| "fields": [] if full else _fields(False), | ||
| } | ||
|
|
||
| if next_page_token: | ||
| body["nextPageToken"] = next_page_token | ||
|
|
||
| logger.debug("Fetching JIRA issues, max=%d, nextPageToken=%s", max_results, next_page_token) | ||
| response_data = jira_api_post(JIRA_SEARCH_PATH, json=body, decode_response=True) | ||
| logger.debug("Got %d issues", len(response_data["issues"])) | ||
|
|
||
| for issue_data in response_data["issues"]: | ||
| yield decode_issue(issue_data, full) | ||
|
|
||
| next_page_token = response_data.get("nextPageToken") | ||
| if not next_page_token or len(response_data["issues"]) == 0: | ||
| break | ||
| if response_data.get("isLast", True): | ||
| break | ||
| next_page_token = response_data.get("nextPageToken") |
There was a problem hiding this comment.
This loop appears to be broken due to incomplete refactoring:
- The
response_datavariable is used but never assigned because thejira_api_postcall has been removed. This will cause anUnboundLocalError. - The pagination logic is broken. The
next_page_tokenis retrieved but not passed in the body of the subsequent request, which would likely cause an infinite loop if there are multiple pages of results. next_page_tokenis assigned twice (lines 391 and 396), which is redundant.
Please restore the API call and fix the pagination logic.
supervisor/jira_utils.py
Outdated
| if is_jira_cloud(): | ||
| users = jira_api_get("user/search", params={"query": email}, api_version="3") | ||
| else: | ||
| users = jira_api_get("user/search", params={"username": email}) |
There was a problem hiding this comment.
According to the comment at the top of the file, Jira API v3 only exists on Jira Cloud. For Jira Server, you must use v2. This call does not specify the API version, so it will default to '3' and likely fail on Jira Server.
| users = jira_api_get("user/search", params={"username": email}) | |
| users = jira_api_get("user/search", params={"username": email}, api_version="2") |
scripts/test_jira_cloud_uat.py
Outdated
| print("UAT Test Script") | ||
|
|
||
| # Test 1: Get custom fields | ||
| print("\n[1/14] Get custom fields") |
There was a problem hiding this comment.
The total number of tests in the progress message is inconsistent. This one says 1/14, but later tests say x/15. Please update this and the other occurrences on lines 53 and 64 to use /15 for consistency.
print("\n[1/15] Get custom fields")
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new UAT test script for Jira Cloud API interactions, covering a wide range of functionalities including issue creation, comment management, label handling, status changes, attachments, JQL searches, and Jotnar tag integration. The core changes in supervisor/jira_utils.py involve refactoring Jira API calls to explicitly support both Jira Cloud and Server by introducing an api_version parameter, using Jira API v3 for JQL and user searches, and retaining v2 for issue, comment, and attachment operations to ensure plain text content. Review comments highlight a critical issue in the new UAT script where test failures are not correctly reported, an unused os import, and an inconsistency in the displayed test count in the log output.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new UAT test script for the Jira Cloud API and refactors the jira_utils.py module to explicitly manage Jira API versions (v2 vs v3). The changes ensure that v2 is used for operations like issue creation, comments, and attachments to get plain text responses, while v3 is used for specific cloud endpoints like JQL search and user search. The review highlights inconsistencies in the default api_version parameters for jira_api_get, jira_api_post, and jira_api_put functions, which currently default to v3 despite the stated preference for v2. It also points out a potential issue in get_user_account_id where it might return a display name instead of the actual account ID. Furthermore, the review suggests improving the UAT test script's determinism and maintainability by replacing datetime.now() with fixed timestamps and defining magic numbers (for sleep duration and max results) as named constants.
Originally implemented by Keerthana in packit#361 Signed-off-by: Tomas Tomecek <ttomecek@redhat.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new UAT test script for the Jira Cloud API, exercising various functionalities like issue creation, commenting, labeling, status changes, attachments, and searching. The core changes in jira_utils.py involve refactoring API calls to explicitly specify Jira API versions (v2 for plain text content, v3 for JQL searches and user lookups) to ensure compatibility and correct data handling. Review feedback highlights critical issues in the UAT script, such as incorrect usage of the @with_requests_session() decorator and blocking time.sleep() calls within asynchronous functions, which should be replaced with asyncio.sleep(). Further improvements suggested include prioritizing accountId for user identification in Jira API calls for better stability, avoiding runtime modification of sys.path, and refactoring a large string literal in the test script for improved readability.
2 participants
Originally implemented by Keerthana in #361
I am carrying this over so we can get all the benefits from #361.
The rebase is not complete, I'll continue tomorrow and make sure the tests pass.Rebase is finished, the testing script works, ready to be reviewed and merged.