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

feat: use bench python to validate python syntax in builds #2301

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
107 changes: 53 additions & 54 deletions press/press/doctype/app_release/app_release.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
# -*- coding: utf-8 -*-
# Copyright (c) 2020, Frappe and contributors
# For license information, please see license.txt

Expand All @@ -11,25 +10,25 @@

import frappe
from frappe.model.document import Document

from press.api.github import get_access_token
from press.press.doctype.app_source.app_source import AppSource
from press.utils import log_error

AppReleaseDict = TypedDict(
"AppReleaseDict",
name=str,
source=str,
hash=str,
cloned=int,
clone_directory=str,
timestamp=Optional[datetime],
creation=datetime,
)
AppReleasePair = TypedDict(
"AppReleasePair",
old=AppReleaseDict,
new=AppReleaseDict,
)

class AppReleaseDict(TypedDict):
name: str
source: str
hash: str
cloned: int
clone_directory: str
timestamp: Optional[datetime] # noqa
creation: datetime


class AppReleasePair(TypedDict):
old: AppReleaseDict
new: AppReleaseDict


class AppRelease(Document):
Expand All @@ -42,23 +41,23 @@
from frappe.types import DF

app: DF.Link
author: DF.Data | None
clone_directory: DF.Text | None
author: DF.Data | None # noqa
clone_directory: DF.Text | None # noqa
cloned: DF.Check
code_server_url: DF.Text | None
code_server_url: DF.Text | None # noqa
hash: DF.Data
invalid_release: DF.Check
invalidation_reason: DF.Code | None
message: DF.Code | None
output: DF.Code | None
invalidation_reason: DF.Code | None # noqa
message: DF.Code | None # noqa
output: DF.Code | None # noqa
public: DF.Check
source: DF.Link
status: DF.Literal["Draft", "Approved", "Awaiting Approval", "Rejected"]
team: DF.Link
timestamp: DF.Datetime | None
timestamp: DF.Datetime | None # noqa
# end: auto-generated types

dashboard_fields = ["app", "source", "message", "hash", "author", "status"]
dashboard_fields = ["app", "source", "message", "hash", "author", "status"] # noqa

@staticmethod
def get_list_query(query, filters=None, **list_args):
Expand Down Expand Up @@ -90,17 +89,15 @@
approval_request_name.as_("approval_request_name"),
)

return query
return query # noqa

Check warning on line 92 in press/press/doctype/app_release/app_release.py

View check run for this annotation

Codecov / codecov/patch

press/press/doctype/app_release/app_release.py#L92

Added line #L92 was not covered by tests

def validate(self):
if not self.clone_directory:
self.set_clone_directory()

def before_save(self):
apps = frappe.get_all("Featured App", {"parent": "Marketplace Settings"}, pluck="app")
teams = frappe.get_all(
"Auto Release Team", {"parent": "Marketplace Settings"}, pluck="team"
)
teams = frappe.get_all("Auto Release Team", {"parent": "Marketplace Settings"}, pluck="team")
if self.team in teams or self.app in apps:
self.status = "Approved"

Expand Down Expand Up @@ -132,16 +129,12 @@
self.save(ignore_permissions=True)

def validate_repo(self):
if (
self.invalid_release
or not self.clone_directory
or not os.path.isdir(self.clone_directory)
):
if self.invalid_release or not self.clone_directory or not os.path.isdir(self.clone_directory):

Check warning on line 132 in press/press/doctype/app_release/app_release.py

View check run for this annotation

Codecov / codecov/patch

press/press/doctype/app_release/app_release.py#L132

Added line #L132 was not covered by tests
return

if syntax_error := check_python_syntax(self.clone_directory):
self.set_invalid(syntax_error)
elif syntax_error := check_pyproject_syntax(self.clone_directory):
if (syntax_error := check_python_syntax(self.clone_directory)) or (

Check warning on line 135 in press/press/doctype/app_release/app_release.py

View check run for this annotation

Codecov / codecov/patch

press/press/doctype/app_release/app_release.py#L135

Added line #L135 was not covered by tests
syntax_error := check_pyproject_syntax(self.clone_directory)
):
self.set_invalid(syntax_error)

def set_invalid(self, reason: str):
Expand All @@ -163,9 +156,7 @@

def set_clone_directory(self):
clone_directory = frappe.db.get_single_value("Press Settings", "clone_directory")
self.clone_directory = os.path.join(
clone_directory, self.app, self.source, self.hash[:10]
)
self.clone_directory = os.path.join(clone_directory, self.app, self.source, self.hash[:10])

def _set_prepared_clone_directory(self, delete_if_exists: bool = False):
self.clone_directory = get_prepared_clone_directory(
Expand All @@ -177,7 +168,9 @@

def _set_code_server_url(self) -> None:
code_server = frappe.db.get_single_value("Press Settings", "code_server")
code_server_url = f"{code_server}/?folder=/home/coder/project/{self.app}/{self.source}/{self.hash[:10]}"
code_server_url = (

Check warning on line 171 in press/press/doctype/app_release/app_release.py

View check run for this annotation

Codecov / codecov/patch

press/press/doctype/app_release/app_release.py#L171

Added line #L171 was not covered by tests
f"{code_server}/?folder=/home/coder/project/{self.app}/{self.source}/{self.hash[:10]}"
)
self.code_server_url = code_server_url

def _clone_repo(self):
Expand Down Expand Up @@ -216,7 +209,7 @@
- If token is not received _get_repo_url throws
- Hence token was received, but app still cannot be cloned
"""
raise Exception("Repository could not be fetched", self.app)
raise Exception("Repository could not be fetched", self.app) # noqa

Check warning on line 212 in press/press/doctype/app_release/app_release.py

View check run for this annotation

Codecov / codecov/patch

press/press/doctype/app_release/app_release.py#L212

Added line #L212 was not covered by tests

self.output += self.run(f"git checkout {self.hash}")
self.output += self.run(f"git reset --hard {self.hash}")
Expand Down Expand Up @@ -355,10 +348,7 @@

team = get_current_team()

return (
f"(`tabApp Release`.`team` = {frappe.db.escape(team)} or `tabApp"
" Release`.`public` = 1)"
)
return f"(`tabApp Release`.`team` = {frappe.db.escape(team)} or `tabApp" " Release`.`public` = 1)"

Check warning on line 351 in press/press/doctype/app_release/app_release.py

View check run for this annotation

Codecov / codecov/patch

press/press/doctype/app_release/app_release.py#L351

Added line #L351 was not covered by tests


def has_permission(doc, ptype, user):
Expand Down Expand Up @@ -411,7 +401,7 @@

def get_changed_files_between_hashes(
source: str, deployed_hash: str, update_hash: str
) -> Optional[tuple[list[str], AppReleasePair]]:
) -> Optional[tuple[list[str], AppReleasePair]]: # noqa
"""
Checks diff between two App Releases, if they have not been cloned
the App Releases are cloned this is because the commit needs to be
Expand Down Expand Up @@ -477,9 +467,7 @@
return releases[0]


def is_update_after_deployed(
update_release: AppReleaseDict, deployed_release: AppReleaseDict
) -> bool:
def is_update_after_deployed(update_release: AppReleaseDict, deployed_release: AppReleaseDict) -> bool:
update_timestamp = update_release["timestamp"]
deployed_timestamp = deployed_release["timestamp"]
if update_timestamp and deployed_timestamp:
Expand All @@ -489,9 +477,7 @@


def run(command, cwd):
return subprocess.check_output(
shlex.split(command), stderr=subprocess.STDOUT, cwd=cwd
).decode()
return subprocess.check_output(shlex.split(command), stderr=subprocess.STDOUT, cwd=cwd).decode()

Check warning on line 480 in press/press/doctype/app_release/app_release.py

View check run for this annotation

Codecov / codecov/patch

press/press/doctype/app_release/app_release.py#L480

Added line #L480 was not covered by tests


def check_python_syntax(dirpath: str) -> str:
Expand All @@ -505,8 +491,8 @@
- -q: quiet, only print errors (stdout)
- -o: optimize level, 0 is no optimization
"""

command = f"python3 -m compileall -q -o 0 {dirpath}"
_python = _get_python_path()
command = f"{_python} -m compileall -q -o 0 {dirpath}"

Check warning on line 495 in press/press/doctype/app_release/app_release.py

View check run for this annotation

Codecov / codecov/patch

press/press/doctype/app_release/app_release.py#L494-L495

Added lines #L494 - L495 were not covered by tests
proc = subprocess.run(
shlex.split(command),
text=True,
Expand All @@ -521,6 +507,19 @@
return proc.stdout


def _get_python_path() -> str:
from frappe.utils import get_bench_path

Check warning on line 511 in press/press/doctype/app_release/app_release.py

View check run for this annotation

Codecov / codecov/patch

press/press/doctype/app_release/app_release.py#L511

Added line #L511 was not covered by tests

bench_path = get_bench_path()

Check warning on line 513 in press/press/doctype/app_release/app_release.py

View check run for this annotation

Codecov / codecov/patch

press/press/doctype/app_release/app_release.py#L513

Added line #L513 was not covered by tests

_python_path = f"{bench_path}/env/bin/python3"

Check warning on line 515 in press/press/doctype/app_release/app_release.py

View check run for this annotation

Codecov / codecov/patch

press/press/doctype/app_release/app_release.py#L515

Added line #L515 was not covered by tests

if not os.path.exists(_python):
_python_path = "python3"

Check warning on line 518 in press/press/doctype/app_release/app_release.py

View check run for this annotation

Codecov / codecov/patch

press/press/doctype/app_release/app_release.py#L517-L518

Added lines #L517 - L518 were not covered by tests

return _python_path

Check warning on line 520 in press/press/doctype/app_release/app_release.py

View check run for this annotation

Codecov / codecov/patch

press/press/doctype/app_release/app_release.py#L520

Added line #L520 was not covered by tests


def check_pyproject_syntax(dirpath: str) -> str:
# tomllib does not report errors as expected
# instead returns empty dict
Expand Down
Loading