Skip to content

Address some linter warnings in submission module #472

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.

Sign up for GitHub

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

Merged
mgoerens merged 1 commit into from
Oct 28, 2025
Merged

Address some linter warnings in submission module #472

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 6 additions & 6 deletions scripts/src/submission/serializer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,23 @@


class SubmissionEncoder(json.JSONEncoder):
def default(self, obj):
if isinstance(obj, submission.Submission):
obj_dict = copy.deepcopy(obj.__dict__)
def default(self, o):
if isinstance(o, submission.Submission):
obj_dict = copy.deepcopy(o.__dict__)
obj_dict["chart"] = obj_dict["chart"].__dict__
obj_dict["report"] = obj_dict["report"].__dict__
obj_dict["source"] = obj_dict["source"].__dict__
obj_dict["tarball"] = obj_dict["tarball"].__dict__
return obj_dict

return json.JSONEncoder.default(self, obj)
return json.JSONEncoder.default(self, o)


class SubmissionDecoder(json.JSONDecoder):
def __init__(self, *args, **kwargs):
json.JSONDecoder.__init__(self, object_hook=self.object_hook, *args, **kwargs)
json.JSONDecoder.__init__(self, object_hook=self.custom_hook, *args, **kwargs)

def object_hook(self, dct):
def custom_hook(self, dct):
if "chart" in dct:
chart_obj = submission.Chart(**dct["chart"])
report_obj = submission.Report(**dct["report"])
Expand Down
46 changes: 24 additions & 22 deletions scripts/src/submission/submission.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from dataclasses import dataclass, field
import os
import re
import tarfile

import requests
import semver
import yaml
Expand All @@ -10,7 +12,6 @@
except ImportError:
from yaml import Loader

from dataclasses import dataclass, field

from owners import owners_file
from tools import gitutils
Expand All @@ -19,44 +20,45 @@

xRateLimit = "X-RateLimit-Limit"
xRateRemain = "X-RateLimit-Remaining"
REQUEST_TIMEOUT = 10


class SubmissionError(Exception):
"""Root Exception for handling any error with the submission"""

pass
"""Root exception for handling any error with the submission"""


class DuplicateChartError(SubmissionError):
"""This Exception is to be raised when the user attempts to submit a PR with more than one chart"""

pass
"""This exception is raised when the user attempts to submit a PR with more than one chart"""


class VersionError(SubmissionError):
"""This Exception is to be raised when the version of the chart is not semver compatible"""

pass
"""This exception is raised when the version of the chart is not semver compatible"""


class WebCatalogOnlyError(SubmissionError):
pass
"""This exception is raised when there is an error when determining the WebCatalogOnly attribute
or if it is inconsistently set

"""


class HelmIndexError(SubmissionError):
pass
"""This exception is raised when the chart is already present in the index, or if there was an error
downloading or reading the helm index

"""


class ReleaseTagError(SubmissionError):
pass
"""This exception is raised when the chart's release already exists"""


class ChartError(SubmissionError):
pass
"""This exception is raised when the redhat prefix is incorrectly set"""


class TarballContentError(SubmissionError):
pass
"""This exception is raised when checking the provided tarball content"""


@dataclass
Expand Down Expand Up @@ -177,7 +179,7 @@ def check_release_tag(self, repository: str):
"Authorization": f'Bearer {os.environ.get("BOT_TOKEN")}',
}
print(f"[INFO] checking tag: {tag_api}")
r = requests.head(tag_api, headers=headers)
r = requests.head(tag_api, headers=headers, timeout=REQUEST_TIMEOUT)
if r.status_code == 200:
msg = f"[ERROR] Helm chart release already exists in the GitHub Release/Tag: {tag_name}"
raise ReleaseTagError(msg)
Expand Down Expand Up @@ -232,10 +234,10 @@ class Submission:

api_url: str
modified_files: list[str] = None
chart: Chart = field(default_factory=lambda: Chart())
report: Report = field(default_factory=lambda: Report())
source: Source = field(default_factory=lambda: Source())
tarball: Tarball = field(default_factory=lambda: Tarball())
chart: Chart = field(default_factory=Chart)
report: Report = field(default_factory=Report)
source: Source = field(default_factory=Source)
tarball: Tarball = field(default_factory=Tarball)
modified_owners: list[str] = field(default_factory=list)
modified_unknown: list[str] = field(default_factory=list)
is_web_catalog_only: bool = None
Expand Down Expand Up @@ -274,7 +276,7 @@ def _get_modified_files(self):
"get", files_api_query, os.environ.get("BOT_TOKEN")
)
except SystemExit as e:
raise SubmissionError(e)
raise SubmissionError(e) from e

files = r.json()
page_size = len(files)
Expand Down Expand Up @@ -643,7 +645,7 @@ def download_index_data(

"""
index_url = f"https://raw.githubusercontent.com/{repository}/{branch}/index.yaml"
r = requests.get(index_url)
r = requests.get(index_url, timeout=REQUEST_TIMEOUT)

data = {"apiVersion": "v1", "entries": {}}
if r.status_code != 200:
Expand Down
100 changes: 52 additions & 48 deletions scripts/src/submission/submission_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,17 @@
"""

import contextlib
import pytest
from dataclasses import dataclass, field
import os
from pathlib import Path
import re
import responses
import tarfile
import tempfile

from dataclasses import dataclass, field
from reporegex import matchers
import pytest
import responses

from pathlib import Path
from reporegex import matchers
from submission import submission

# Define assets that are being reused accross tests
Expand Down Expand Up @@ -132,10 +132,10 @@ def create_tarball(tarball_basedir: str, tarball_name: str, tarball_content: lis
class SubmissionInitScenario:
api_url: str
modified_files: list[str]
tarball_content: list[str] = field(default_factory=lambda: list())
tarball_content: list[str] = field(default_factory=list)
expected_submission: submission.Submission = None
excepted_exception: contextlib.ContextDecorator = field(
default_factory=lambda: contextlib.nullcontext()
default_factory=contextlib.nullcontext
)


Expand Down Expand Up @@ -583,7 +583,7 @@ class WebCatalogOnlyScenario:
report_web_catalog_only: str = None # Set to None to skip key creation in report
expected_output: bool = None
excepted_exception: contextlib.ContextDecorator = field(
default_factory=lambda: contextlib.nullcontext()
default_factory=contextlib.nullcontext
)


Expand Down Expand Up @@ -706,34 +706,36 @@ def test_parse_web_catalog_only(test_scenario):

# Populate OWNERS file
if test_scenario.create_owners:
owners_file = open(os.path.join(owners_base_path, "OWNERS"), "w")
owners_file.write(
"publicPgpKey: unknown"
) # Make sure OWNERS is not an empty file
if test_scenario.owners_web_catalog_only:
with open(
os.path.join(owners_base_path, "OWNERS"), "w", encoding="utf-8"
) as owners_file:
owners_file.write(
f"\nproviderDelivery: {test_scenario.owners_web_catalog_only}"
)
owners_file.close()
"publicPgpKey: unknown"
) # Make sure OWNERS is not an empty file
if test_scenario.owners_web_catalog_only:
owners_file.write(
f"\nproviderDelivery: {test_scenario.owners_web_catalog_only}"
)

# Populate report.yaml file
if test_scenario.create_report:
report_file = open(os.path.join(chart_base_path, "report.yaml"), "w")
report_file.writelines(
[
"apiversion: v1",
"\nkind: verify-report",
]
)
if test_scenario.report_web_catalog_only:
with open(
os.path.join(chart_base_path, "report.yaml"), "w", encoding="utf-8"
) as report_file:
report_file.writelines(
[
"\nmetadata:",
"\n tool:",
f"\n webCatalogOnly: {test_scenario.report_web_catalog_only}",
"apiversion: v1",
"\nkind: verify-report",
]
)
report_file.close()
if test_scenario.report_web_catalog_only:
report_file.writelines(
[
"\nmetadata:",
"\n tool:",
f"\n webCatalogOnly: {test_scenario.report_web_catalog_only}",
]
)

with test_scenario.excepted_exception:
test_scenario.input_submission.parse_web_catalog_only(repo_path=temp_dir)
Expand Down Expand Up @@ -818,23 +820,24 @@ def test_is_valid_web_catalog_only(test_scenario):

# Populate report.yaml file
if test_scenario.create_report:
report_file = open(os.path.join(chart_base_path, "report.yaml"), "w")
report_file.writelines(
[
"apiversion: v1",
"\nkind: verify-report",
]
)
if test_scenario.report_has_digest:
with open(
os.path.join(chart_base_path, "report.yaml"), "w", encoding="utf-8"
) as report_file:
report_file.writelines(
[
"\nmetadata:",
"\n tool:",
"\n digests:",
"\n package: 7755e7cf43e55bbf2edafd9788b773b844fb15626c5ff8ff7a30a6d9034f3a33",
"apiversion: v1",
"\nkind: verify-report",
]
)
report_file.close()
if test_scenario.report_has_digest:
report_file.writelines(
[
"\nmetadata:",
"\n tool:",
"\n digests:",
"\n package: 7755e7cf43e55bbf2edafd9788b773b844fb15626c5ff8ff7a30a6d9034f3a33",
]
)

is_valid_web_catalog_only, reason = (
test_scenario.input_submission.is_valid_web_catalog_only(repo_path=temp_dir)
Expand All @@ -846,12 +849,15 @@ def test_is_valid_web_catalog_only(test_scenario):
assert test_scenario.expected_reason in reason


def create_new_index(charts: list[submission.Chart] = []):
def create_new_index(charts: list[submission.Chart] = None):
"""Create the JSON representation of a Helm chart index containing the provided list of charts

The resulting index only contains the required information for the check_index to work.

"""
if charts is None:
charts = []

index = {"apiVersion": "v1", "entries": {}}

for chart in charts:
Expand All @@ -878,9 +884,9 @@ class CheckIndexScenario:
version=expected_version,
)
)
index: dict = field(default_factory=lambda: create_new_index())
index: dict = field(default_factory=create_new_index)
excepted_exception: contextlib.ContextDecorator = field(
default_factory=lambda: contextlib.nullcontext()
default_factory=contextlib.nullcontext
)


Expand Down Expand Up @@ -931,9 +937,9 @@ class CheckReleaseTagScenario:
version=expected_version,
)
)
exising_tags: list[str] = field(default_factory=lambda: list())
exising_tags: list[str] = field(default_factory=list)
excepted_exception: contextlib.ContextDecorator = field(
default_factory=lambda: contextlib.nullcontext()
default_factory=contextlib.nullcontext
)


Expand Down Expand Up @@ -965,15 +971,13 @@ def test_check_release_tag(test_scenario):
if chart_release_tag not in test_scenario.exising_tags:
responses.head(
f"https://api.github.com/repos/my-fake-org/my-fake-repo/git/ref/tags/{chart_release_tag}",
# json=[{"filename": file} for file in test_scenario.modified_files],
status=404,
)

for tag in test_scenario.exising_tags:
# Mock GitHub API
responses.head(
f"https://api.github.com/repos/my-fake-org/my-fake-repo/git/ref/tags/{tag}",
# json=[{"filename": file} for file in test_scenario.modified_files],
)

with test_scenario.excepted_exception:
Expand Down
6 changes: 3 additions & 3 deletions scripts/src/submission/validate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ def write_submission_to_file(s: submission.Submission, artifact_path: str):
"""Save a JSON representation of the Submission object to file."""
data = serializer.SubmissionEncoder().encode(s)

with open(artifact_path, "w") as f:
with open(artifact_path, "w", encoding="utf-8") as f:
f.write(data)


def read_submission_from_file(articact_path: str) -> submission.Submission:
"""Read and load the JSON representation of the Submission object from file."""
with open(articact_path, "r") as f:
with open(articact_path, "r", encoding="utf-8") as f:
s = json.load(f, cls=serializer.SubmissionDecoder)

return s
Expand All @@ -44,7 +44,7 @@ def craft_pr_content_error_msg(
try:
s.parse_modified_files(repo_path="pr-branch")
except submission.SubmissionError as e:
raise ParseFilesError(str(e))
raise ParseFilesError(str(e)) from e

# Checks that this PR is a valid "Chart certification" PR
is_valid, msg = s.is_valid_certification_submission(ignore_owners=True)
Expand Down