Skip to content
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

Update comment #91

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .flake8
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ ignore = W503
max-complexity = 10
max-line-length = 120
select = E,W,F,C,N
exclude = .env,env,venv,.venv
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,8 @@
__pycache__/
.coverage
/coverage.xml
.env/
.venv/
venv/
env/
.vscode/
5 changes: 5 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,8 @@
.licenses/
__pycache__/
node_modules/
.env/
.venv/
env/
venv/
.mypy_cache/
3 changes: 3 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ inputs:
github-token:
description: "GitHub access token used to comment the memory usage comparison results to the PR thread"
default: ${{ github.token }}
update-comment:
description: "when posting a report, set this to true to only update the previous report (if one exists)"
default: false
runs:
using: "docker"
image: "Dockerfile"
4 changes: 2 additions & 2 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ description = ""
authors = ["Arduino <info@arduino.cc>"]

[tool.poetry.dependencies]
python = "3.11.*"
python = ">=3.11"

[tool.poetry.group.dev.dependencies]
black = "24.3.0"
Expand Down
59 changes: 44 additions & 15 deletions reportsizedeltas/reportsizedeltas.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ def main() -> None:
repository_name=os.environ["GITHUB_REPOSITORY"],
sketches_reports_source=os.environ["INPUT_SKETCHES-REPORTS-SOURCE"],
token=os.environ["INPUT_GITHUB-TOKEN"],
update_comment=os.environ["INPUT_UPDATE-COMMENT"] == "true",
)

report_size_deltas.report_size_deltas()
Expand All @@ -47,7 +48,7 @@ def set_verbosity(enable_verbosity: bool) -> None:
# INFO: manually specified output and all higher log level output
verbose_logging_level = logging.DEBUG

if type(enable_verbosity) is not bool:
if not isinstance(enable_verbosity, bool):
raise TypeError
if enable_verbosity:
logger.setLevel(level=verbose_logging_level)
Expand Down Expand Up @@ -86,10 +87,11 @@ class ReportKeys:
sketches = "sketches"
compilation_success = "compilation_success"

def __init__(self, repository_name: str, sketches_reports_source: str, token: str) -> None:
def __init__(self, repository_name: str, sketches_reports_source: str, token: str, update_comment: bool) -> None:
self.repository_name = repository_name
self.sketches_reports_source = sketches_reports_source
self.token = token
self.update_comment = update_comment

def report_size_deltas(self) -> None:
"""Comment a report of memory usage change to pull request(s)."""
Expand Down Expand Up @@ -167,33 +169,53 @@ def report_size_deltas_from_workflow_artifacts(self) -> None:
page_number += 1
page_count = api_data["page_count"]

def report_exists(self, pr_number: int, pr_head_sha: str) -> bool:
def report_exists(self, pr_number: int, pr_head_sha: str = "") -> str | None:
"""Return whether a report has already been commented to the pull request thread for the latest workflow run.

Keyword arguments:
pr_number -- number of the pull request to check
pr_head_sha -- PR's head branch hash
If `pr_head_sha` is a blank str, then all thread comments are traversed. Additionally,
any report comment found will be deleted if it is not the first report comment found.
This is designed to support the action input `update-comment` when asserted.

If `pr_head_sha` is not a blank str, then thread comments are traversed
until a report comment that corresponds to the commit is found.
No comments are deleted in this scenario.

Arguments:
pr_number: number of the pull request to check
pr_head_sha: PR's head branch hash
Returns:
- A URL str to use for PATCHing the first applicable report comment.
- None if no applicable report comments exist.
"""
# Get the pull request's comments
comment_url: str | None = None
request_uri = f"repos/{self.repository_name}/issues/{pr_number}/comments"
page_number = 1
page_count = 1
while page_number <= page_count:
# Get the pull request's comments
api_data = self.api_request(
request="repos/" + self.repository_name + "/issues/" + str(pr_number) + "/comments",
request=request_uri,
page_number=page_number,
)

comments_data = api_data["json_data"]
for comment_data in comments_data:
# Check if the comment is a report for the PR's head SHA
if comment_data["body"].startswith(self.report_key_beginning + pr_head_sha):
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only improvement I can think of is prepending self.report_key_beginning with an HTML comment:

report_key_beginning = "**Memory usage change @ "

where as using

    report_key_beginning = "<!-- arduino/report-size-deltas -->\n**Memory usage change @ "

would better assure that this action doesn't attempt to alter some user's comment starting with **Memory usage change @ .

return True
if pr_head_sha:
return comment_data["url"]
# else: pr_head_sha == ""
if comment_url is None:
comment_url = comment_data["url"]
else: # found another report
# delete report comment if it is not the first report found
self.http_request(url=comment_data["url"], method="DELETE")

page_number += 1
page_count = api_data["page_count"]

# No reports found for the PR's head SHA
return False
return comment_url

def get_artifacts_data_for_sha(self, pr_user_login: str, pr_head_ref: str, pr_head_sha: str):
"""Return the list of data objects for the report artifacts associated with the given head commit hash.
Expand Down Expand Up @@ -534,7 +556,10 @@ def comment_report(self, pr_number: int, report_markdown: str) -> None:
report_data = json.dumps(obj={"body": report_markdown}).encode(encoding="utf-8")
url = "https://api.github.com/repos/" + self.repository_name + "/issues/" + str(pr_number) + "/comments"

self.http_request(url=url, data=report_data)
comment_url = None if not self.update_comment else self.report_exists(pr_number=pr_number)
method = "PATCH" if comment_url else None

self.http_request(url=comment_url or url, data=report_data, method=method)

def api_request(self, request: str, request_parameters: str = "", page_number: int = 1):
"""Do a GitHub API request. Return a dictionary containing:
Expand Down Expand Up @@ -594,7 +619,7 @@ def get_json_response(self, url: str):
except Exception as exception:
raise exception

def http_request(self, url: str, data: bytes | None = None):
def http_request(self, url: str, data: bytes | None = None, method: str | None = None):
"""Make a request and return a dictionary:
read -- the response
info -- headers
Expand All @@ -604,28 +629,32 @@ def http_request(self, url: str, data: bytes | None = None):
url -- the URL to load
data -- data to pass with the request
(default value: None)
method -- the HTTP request method to use
(default is None which means ``'GET' if data is None else 'POST'``).
"""
with self.raw_http_request(url=url, data=data) as response_object:
with self.raw_http_request(url=url, data=data, method=method) as response_object:
return {
"body": response_object.read().decode(encoding="utf-8", errors="ignore"),
"headers": response_object.info(),
"url": response_object.geturl(),
}

def raw_http_request(self, url: str, data: bytes | None = None):
def raw_http_request(self, url: str, data: bytes | None = None, method: str | None = None):
"""Make a request and return an object containing the response.

Keyword arguments:
url -- the URL to load
data -- data to pass with the request
(default value: None)
method -- the HTTP request method to use
(default is None which means ``'GET' if data is None else 'POST'``).
"""
# Maximum times to retry opening the URL before giving up
maximum_urlopen_retries = 3

logger.info("Opening URL: " + url)

request = urllib.request.Request(url=url, data=data)
request = urllib.request.Request(url=url, data=data, method=method)
request.add_unredirected_header(key="Accept", val="application/vnd.github+json")
request.add_unredirected_header(key="Authorization", val="Bearer " + self.token)
request.add_unredirected_header(key="User-Agent", val=self.repository_name.split("/")[0])
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
[
{
"url": "https://api.github.com/repos/test-user/test-repo/issues/comments/1953679626",
"body": "**Memory usage change @ 525def8e855e5f227a4b4a0b6f84c34cd288a5ea**\n\nBoard|flash|%|RAM for global variables|%\n-|-|-|-|-\n`arduino:avr:nano`|0 - 0|0.0 - 0.0|0 - 0|0.0 - 0.0\n`arduino:samd:mkrzero`|0 - 0|0.0 - 0.0|0 - 0|0.0 - 0.0\n\n<details>\n<summary>Click for full report table</summary>\n\nBoard|`examples/Getting_Started_SimpleClient_Mesh`<br>flash|%|`examples/Getting_Started_SimpleClient_Mesh`<br>RAM for global variables|%|`examples/Getting_Started_SimpleServer_Mesh`<br>flash|%|`examples/Getting_Started_SimpleServer_Mesh`<br>RAM for global variables|%|`examples/InteractiveServer_Mesh`<br>flash|%|`examples/InteractiveServer_Mesh`<br>RAM for global variables|%|`examples/MQTT/mqtt_basic`<br>flash|%|`examples/MQTT/mqtt_basic`<br>RAM for global variables|%|`examples/MQTT/mqtt_basic_2`<br>flash|%|`examples/MQTT/mqtt_basic_2`<br>RAM for global variables|%|`examples/SimpleClient_Mesh`<br>flash|%|`examples/SimpleClient_Mesh`<br>RAM for global variables|%\n-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-\n`arduino:avr:nano`|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0\n`arduino:samd:mkrzero`|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0\n\n</details>\n\n<details>\n<summary>Click for full report CSV</summary>\n\n```\nBoard,examples/Getting_Started_SimpleClient_Mesh<br>flash,%,examples/Getting_Started_SimpleClient_Mesh<br>RAM for global variables,%,examples/Getting_Started_SimpleServer_Mesh<br>flash,%,examples/Getting_Started_SimpleServer_Mesh<br>RAM for global variables,%,examples/InteractiveServer_Mesh<br>flash,%,examples/InteractiveServer_Mesh<br>RAM for global variables,%,examples/MQTT/mqtt_basic<br>flash,%,examples/MQTT/mqtt_basic<br>RAM for global variables,%,examples/MQTT/mqtt_basic_2<br>flash,%,examples/MQTT/mqtt_basic_2<br>RAM for global variables,%,examples/SimpleClient_Mesh<br>flash,%,examples/SimpleClient_Mesh<br>RAM for global variables,%\narduino:avr:nano,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0\narduino:samd:mkrzero,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0\n```\n</details>"
},
{
"url": "https://api.github.com/repos/test-user/test-repo/issues/comments/1953996601",
"body": "**Memory usage change @ 862a917fe24c6f737abc336b681b7326c0fcceec**\n\nBoard|flash|%|RAM for global variables|%\n-|-|-|-|-\n`arduino:avr:nano`|0 - 0|0.0 - 0.0|0 - 0|0.0 - 0.0\n`arduino:samd:mkrzero`|0 - 0|0.0 - 0.0|0 - 0|0.0 - 0.0\n\n<details>\n<summary>Click for full report table</summary>\n\nBoard|`examples/Getting_Started_SimpleClient_Mesh`<br>flash|%|`examples/Getting_Started_SimpleClient_Mesh`<br>RAM for global variables|%|`examples/Getting_Started_SimpleServer_Mesh`<br>flash|%|`examples/Getting_Started_SimpleServer_Mesh`<br>RAM for global variables|%|`examples/InteractiveServer_Mesh`<br>flash|%|`examples/InteractiveServer_Mesh`<br>RAM for global variables|%|`examples/MQTT/mqtt_basic`<br>flash|%|`examples/MQTT/mqtt_basic`<br>RAM for global variables|%|`examples/MQTT/mqtt_basic_2`<br>flash|%|`examples/MQTT/mqtt_basic_2`<br>RAM for global variables|%|`examples/SimpleClient_Mesh`<br>flash|%|`examples/SimpleClient_Mesh`<br>RAM for global variables|%\n-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-\n`arduino:avr:nano`|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0\n`arduino:samd:mkrzero`|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0\n\n</details>\n\n<details>\n<summary>Click for full report CSV</summary>\n\n```\nBoard,examples/Getting_Started_SimpleClient_Mesh<br>flash,%,examples/Getting_Started_SimpleClient_Mesh<br>RAM for global variables,%,examples/Getting_Started_SimpleServer_Mesh<br>flash,%,examples/Getting_Started_SimpleServer_Mesh<br>RAM for global variables,%,examples/InteractiveServer_Mesh<br>flash,%,examples/InteractiveServer_Mesh<br>RAM for global variables,%,examples/MQTT/mqtt_basic<br>flash,%,examples/MQTT/mqtt_basic<br>RAM for global variables,%,examples/MQTT/mqtt_basic_2<br>flash,%,examples/MQTT/mqtt_basic_2<br>RAM for global variables,%,examples/SimpleClient_Mesh<br>flash,%,examples/SimpleClient_Mesh<br>RAM for global variables,%\narduino:avr:nano,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0\narduino:samd:mkrzero,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0\n```\n</details>"
},
{
"url": "https://api.github.com/repos/test-user/test-repo/issues/comments/1954284300",
"body": "**Memory usage change @ 0098017901c516c6cd49e248f1aabd6b30c91aa9**\n\nBoard|flash|%|RAM for global variables|%\n-|-|-|-|-\n`arduino:avr:nano`|0 - 0|0.0 - 0.0|0 - 0|0.0 - 0.0\n`arduino:samd:mkrzero`|0 - 0|0.0 - 0.0|0 - 0|0.0 - 0.0\n\n<details>\n<summary>Click for full report table</summary>\n\nBoard|`examples/Getting_Started_SimpleClient_Mesh`<br>flash|%|`examples/Getting_Started_SimpleClient_Mesh`<br>RAM for global variables|%|`examples/Getting_Started_SimpleServer_Mesh`<br>flash|%|`examples/Getting_Started_SimpleServer_Mesh`<br>RAM for global variables|%|`examples/InteractiveServer_Mesh`<br>flash|%|`examples/InteractiveServer_Mesh`<br>RAM for global variables|%|`examples/MQTT/mqtt_basic`<br>flash|%|`examples/MQTT/mqtt_basic`<br>RAM for global variables|%|`examples/MQTT/mqtt_basic_2`<br>flash|%|`examples/MQTT/mqtt_basic_2`<br>RAM for global variables|%|`examples/SimpleClient_Mesh`<br>flash|%|`examples/SimpleClient_Mesh`<br>RAM for global variables|%\n-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-\n`arduino:avr:nano`|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0\n`arduino:samd:mkrzero`|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0\n\n</details>\n\n<details>\n<summary>Click for full report CSV</summary>\n\n```\nBoard,examples/Getting_Started_SimpleClient_Mesh<br>flash,%,examples/Getting_Started_SimpleClient_Mesh<br>RAM for global variables,%,examples/Getting_Started_SimpleServer_Mesh<br>flash,%,examples/Getting_Started_SimpleServer_Mesh<br>RAM for global variables,%,examples/InteractiveServer_Mesh<br>flash,%,examples/InteractiveServer_Mesh<br>RAM for global variables,%,examples/MQTT/mqtt_basic<br>flash,%,examples/MQTT/mqtt_basic<br>RAM for global variables,%,examples/MQTT/mqtt_basic_2<br>flash,%,examples/MQTT/mqtt_basic_2<br>RAM for global variables,%,examples/SimpleClient_Mesh<br>flash,%,examples/SimpleClient_Mesh<br>RAM for global variables,%\narduino:avr:nano,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0\narduino:samd:mkrzero,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0\n```\n</details>"
},
{
"url": "https://api.github.com/repos/test-user/test-repo/issues/comments/1955441026",
"body": "**Memory usage change @ f9881f578cf28800a2076ae709434249e9cb9d67**\n\nBoard|flash|%|RAM for global variables|%\n-|-|-|-|-\n`arduino:avr:nano`|0 - 0|0.0 - 0.0|0 - 0|0.0 - 0.0\n`arduino:samd:mkrzero`|0 - 0|0.0 - 0.0|0 - 0|0.0 - 0.0\n\n<details>\n<summary>Click for full report table</summary>\n\nBoard|`examples/Getting_Started_SimpleClient_Mesh`<br>flash|%|`examples/Getting_Started_SimpleClient_Mesh`<br>RAM for global variables|%|`examples/Getting_Started_SimpleServer_Mesh`<br>flash|%|`examples/Getting_Started_SimpleServer_Mesh`<br>RAM for global variables|%|`examples/InteractiveServer_Mesh`<br>flash|%|`examples/InteractiveServer_Mesh`<br>RAM for global variables|%|`examples/MQTT/mqtt_basic`<br>flash|%|`examples/MQTT/mqtt_basic`<br>RAM for global variables|%|`examples/MQTT/mqtt_basic_2`<br>flash|%|`examples/MQTT/mqtt_basic_2`<br>RAM for global variables|%|`examples/SimpleClient_Mesh`<br>flash|%|`examples/SimpleClient_Mesh`<br>RAM for global variables|%\n-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-\n`arduino:avr:nano`|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0\n`arduino:samd:mkrzero`|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0|0|0.0\n\n</details>\n\n<details>\n<summary>Click for full report CSV</summary>\n\n```\nBoard,examples/Getting_Started_SimpleClient_Mesh<br>flash,%,examples/Getting_Started_SimpleClient_Mesh<br>RAM for global variables,%,examples/Getting_Started_SimpleServer_Mesh<br>flash,%,examples/Getting_Started_SimpleServer_Mesh<br>RAM for global variables,%,examples/InteractiveServer_Mesh<br>flash,%,examples/InteractiveServer_Mesh<br>RAM for global variables,%,examples/MQTT/mqtt_basic<br>flash,%,examples/MQTT/mqtt_basic<br>RAM for global variables,%,examples/MQTT/mqtt_basic_2<br>flash,%,examples/MQTT/mqtt_basic_2<br>RAM for global variables,%,examples/SimpleClient_Mesh<br>flash,%,examples/SimpleClient_Mesh<br>RAM for global variables,%\narduino:avr:nano,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0\narduino:samd:mkrzero,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0\n```\n</details>"
},
{
"url": "https://api.github.com/repos/test-user/test-repo/issues/comments/1955465432",
"body": "NOT A BOT COMMENT"
}
]
Loading
Loading