-
-
Notifications
You must be signed in to change notification settings - Fork 104
[feature] Automated follow-up alpha version bump PRs #756
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
f8ce53a
ddaff1a
072340f
0c92e41
71e47b0
fb77d52
8252ea6
b111bb8
6da67ec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import uuid | ||
| from urllib.parse import urlparse | ||
|
|
||
| import requests | ||
| from openwisp_utils.utils import retryable_request as utils_retryable_request | ||
|
|
@@ -38,11 +39,60 @@ def create_pr(self, head, base, title): | |
|
|
||
| def is_pr_merged(self, pr_url): | ||
| """Checks if a pull request has been merged.""" | ||
| pr_number = pr_url.split("/")[-1] | ||
| pr_number = pr_url.rstrip("/").split("/")[-1] | ||
| url = f"{self.base_url}/pulls/{pr_number}" | ||
| response = retryable_request(method="get", url=url, headers=self.headers) | ||
| return response.json().get("merged", False) | ||
|
|
||
| def get_pr(self, pr_url): | ||
| """Returns metadata for a pull request in the configured repository.""" | ||
| parsed_url = urlparse(pr_url) | ||
| path = parsed_url.path.strip("/").split("/") | ||
| if ( | ||
| parsed_url.scheme not in {"http", "https"} | ||
| or parsed_url.netloc != "github.com" | ||
| or len(path) != 4 | ||
| or "/".join(path[:2]) != self.repo | ||
| or path[2] != "pull" | ||
| or not path[3].isdigit() | ||
| ): | ||
| raise ValueError( | ||
| "The pull request URL must belong to the configured GitHub repository." | ||
| ) | ||
| pr_number = path[3] | ||
| url = f"{self.base_url}/pulls/{pr_number}" | ||
| response = retryable_request(method="get", url=url, headers=self.headers) | ||
| return response.json() | ||
|
|
||
| def get_release(self, tag_name): | ||
| """Returns the release for a tag, or None when it has not been created.""" | ||
| response = retryable_request( | ||
| method="get", | ||
| url=f"{self.base_url}/releases/tags/{tag_name}", | ||
| headers=self.headers, | ||
| allowed_status_codes=(404,), | ||
| ) | ||
| if response.status_code == 404: | ||
| return None | ||
| return response.json() | ||
|
|
||
| def find_pr(self, head, base, title): | ||
| """Returns an existing matching pull request, regardless of its state.""" | ||
| response = retryable_request( | ||
| method="get", | ||
| url=f"{self.base_url}/pulls", | ||
| headers=self.headers, | ||
| params={"state": "all", "base": base, "per_page": 100}, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- github.py outline ---'
ast-grep outline openwisp_utils/releaser/github.py
printf '%s\n' '--- github.py relevant source ---'
sed -n '1,180p' openwisp_utils/releaser/github.py
printf '%s\n' '--- find_pr references ---'
rg -n -C 4 'find_pr|create_pull|pulls' openwisp_utils tests 2>/dev/null | head -240Repository: openwisp/openwisp-utils Length of output: 13958 🤖 get_repo_knowledge executed:
Length of output: 6911 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- release caller branches ---'
sed -n '340,410p' openwisp_utils/releaser/release.py
printf '%s\n' '--- find_pr tests ---'
rg -n -C 12 'find_pr|existing matching|per_page' openwisp_utils/releaser/tests
printf '%s\n' '--- releaser retry helper ---'
sed -n '1,180p' openwisp_utils/releaser/utils.py
printf '%s\n' '--- relevant repository conventions ---'
cat /tmp/coderabbit-repo-knowledge/openwisp-openwisp-utils-2837d08b/conventions/repo-wide.md 2>/dev/null || trueRepository: openwisp/openwisp-utils Length of output: 3305 🌐 Web query:
💡 Result: The GitHub REST API endpoint for listing pull requests supports pagination using the per_page and page query parameters [1][2]. Key details regarding pagination: per_page: This parameter controls the number of results returned per page [3][4]. The maximum allowed value for per_page is 100 [1][2]. The default value is 30 [1][2]. If you specify a value greater than 100, the API will automatically reduce it to the maximum of 100 without returning an error [3][4]. page: This parameter specifies the page number of the results to fetch, with a default value of 1 [1][2]. To retrieve all pull requests, you should iterate through the pages using the link header provided in the API response [3][4]. The link header contains URLs for subsequent pages, which the API client can use to continue fetching data until all items have been retrieved [3][4]. Alternatively, many API libraries (such as Octokit) provide helper methods to automatically handle this pagination [3][4]. Citations:
🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- alpha bump continuation ---'
sed -n '410,485p' openwisp_utils/releaser/release.py
printf '%s\n' '--- changelog port caller ---'
sed -n '255,315p' openwisp_utils/releaser/release.py
printf '%s\n' '--- releaser tests inventory ---'
git ls-files 'openwisp_utils/releaser/tests/*' | sortRepository: openwisp/openwisp-utils Length of output: 6572 Paginate
🤖 Prompt for AI Agents |
||
| ) | ||
| for pull_request in response.json(): | ||
| if ( | ||
| pull_request.get("title") == title | ||
| and pull_request.get("head", {}).get("ref") == head | ||
| and pull_request.get("base", {}).get("ref") == base | ||
| ): | ||
| return pull_request | ||
| return None | ||
|
|
||
| def create_release(self, tag_name, title, body): | ||
| """Creates a draft release on GitHub.""" | ||
| url = f"{self.base_url}/releases" | ||
|
|
@@ -54,8 +104,17 @@ def create_release(self, tag_name, title, body): | |
| "prerelease": False, | ||
| } | ||
| response = retryable_request( | ||
| method="post", url=url, headers=self.headers, json=payload | ||
| method="post", | ||
| url=url, | ||
| headers=self.headers, | ||
| json=payload, | ||
| allowed_status_codes=(422,), | ||
| ) | ||
| if response.status_code == 422: | ||
| existing_release = self.get_release(tag_name) | ||
| if existing_release: | ||
| return existing_release["html_url"] | ||
| response.raise_for_status() | ||
| return response.json()["html_url"] | ||
|
|
||
| def check_pr_creation_permission(self) -> tuple[bool, str]: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🌐 Web query:
Python argparse two optional positional arguments nargs="?" assignment order💡 Result:
In Python's argparse, when multiple positional arguments are defined with nargs='?', the parser assigns command-line values to them in the order in which the arguments were defined [1][2]. Because nargs='?' makes a positional argument optional, the parser attempts to consume available command-line values greedily as it processes them in the defined order [3][4]. This behavior often leads to unexpected results if the user intends for later positional arguments to be populated while skipping earlier ones [4]. Key technical considerations include: Greedy Consumption: The parser does not perform complex backtracking to determine the "correct" assignment if multiple interpretations are possible [5]. If an argument defined earlier in the script has nargs='?', it will consume the first available positional value, potentially "stealing" an argument intended for a subsequent positional parameter [4]. Order Sensitivity: Positional arguments are assigned sequentially based on their definition order [1][2]. For example, if you define parser.add_argument('A', nargs='?') followed by parser.add_argument('B', nargs='?'), an input of ['val'] will assign 'val' to A, leaving B with its default value [1][4]. Ambiguity with Optional Flags: Mixing positional arguments that use nargs='?' with optional flags (e.g., --flag) can create significant parsing ambiguity [5]. The parser may misinterpret a flag or its value as a positional argument [3][2]. Recommended Best Practices: Use Clearer Definitions: If you must have multiple optional positional arguments, ensure their usage is unambiguous or use them only at the end of the argument list [4]. Use -- for Separation: You can use the -- separator on the command line to explicitly tell the parser that all subsequent tokens should be treated as positional arguments, helping to resolve ambiguities [6][7]. Consider Alternative Structures: If the logic requires complex optionality, consider using flagged optional arguments instead of positional ones, as they are not restricted by strict positional order and are less prone to consumption conflicts [5][4]. Parse Intermixed Arguments: For specific use cases involving flags intermixed with positionals, Python 3.7+ provides the parse_intermixed_args method, which can help resolve some ordering issues [3].
Citations:
🏁 Script executed:
Repository: openwisp/openwisp-utils
Length of output: 1075
🏁 Script executed:
Repository: openwisp/openwisp-utils
Length of output: 1483
Remove the unreachable validation branch.
argparseassigns the first positional value tocommandand rejects values other than"resume"throughchoices; therefore, after parsing succeeds,args.command is Noneimpliesargs.pr_urlis alsoNone. Theparser.errorcall at line 16 is redundant.🤖 Prompt for AI Agents
Source: Path instructions