Skip to content
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
5 changes: 5 additions & 0 deletions press/api/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -981,6 +981,11 @@ def get_frappe_io_auth_url() -> str | None:
return None


@frappe.whitelist(allow_guest=True)
def frappe_io_login():
return redirect_to(get_frappe_io_auth_url() or "/")


@frappe.whitelist()
def get_emails():
team = get_current_team(get_doc=False)
Expand Down
4 changes: 1 addition & 3 deletions press/hooks.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
from press.api.account import get_frappe_io_auth_url

from . import __version__ as app_version

app_name = "press"
Expand Down Expand Up @@ -69,7 +67,7 @@
]

website_redirects = [
{"source": "/dashboard/f-login", "target": get_frappe_io_auth_url() or "/"},
{"source": "/dashboard/f-login", "target": "/api/method/press.api.account.frappe_io_login"},
{
"source": "/suspended-site",
"target": "/api/method/press.api.handle_suspended_site_redirection",
Expand Down
25 changes: 11 additions & 14 deletions press/press/doctype/bench_shell_log/bench_shell_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,17 @@
import frappe
from frappe.model.document import Document

ExecuteResult = TypedDict(
"ExecuteResult",
{
"command": str,
"status": str,
"start": str,
"end": str,
"duration": float,
"output": str,
"directory": Optional[str],
"traceback": Optional[str],
"returncode": Optional[int],
},
)

class ExecuteResult(TypedDict):
command: str
status: str
start: str
end: str
duration: float
output: str
directory: Optional[str]
traceback: Optional[str]
returncode: Optional[int]


class BenchShellLog(Document):
Expand Down
13 changes: 6 additions & 7 deletions press/press/doctype/deploy_candidate/cache_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,12 @@
from textwrap import dedent
from typing import Tuple, TypedDict

CommandOutput = TypedDict(
"CommandOutput",
cwd=str,
image_tag=str,
returncode=int,
output=str,
)

class CommandOutput(TypedDict):
cwd: str
image_tag: str
returncode: int
output: str


def copy_file_from_docker_cache(
Expand Down
27 changes: 23 additions & 4 deletions press/press/doctype/deploy_candidate/docker_output_parsers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@

import json
import re
import shlex
import typing
from typing import Literal

import dockerfile
import frappe
from frappe.core.utils import find
from frappe.utils import now_datetime, rounded
Expand Down Expand Up @@ -211,9 +211,7 @@ def ansi_escape(text: str) -> str:


def get_command(name: str) -> str:
# Strip docker flags and commands from the line
line = dockerfile.parse_string(name)[0]
command = " ".join(line.value).strip() or line.original.split(maxsplit=1)[1]
command = _get_run_command(name)
command = command.split("`#stage-", maxsplit=1)[0]

# Remove line fold slashes
Expand All @@ -227,6 +225,27 @@ def get_command(name: str) -> str:
return "\n".join([p for p in splits if len(p)])


def _get_run_command(line: str) -> str:
instruction, _, value = line.partition(" ")
if instruction.upper() != "RUN":
return value

value = value.strip()
if value.startswith("["):
return value

try:
parts = shlex.split(value, posix=True)
except ValueError:
return value

for i, part in enumerate(parts):
if not part.startswith("--"):
return shlex.join(parts[i:])

return ""
Comment on lines +228 to +246
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 shlex.join re-encodes the command, breaking downstream splitting

_get_run_command calls shlex.split (which strips quotes and consumes backslash-newline continuations) and then shlex.join (which re-quotes tokens with single quotes). This means: (1) `#stage-xxx` markers get re-quoted as '#stage-xxx', so get_command's split("#stage-")no longer matches them correctly; (2) backslash-newline line folds are consumed by shlex, soget_command's \\nsplit no longer works; (3)"hello world"(double-quoted) becomes'hello world'` (single-quoted) in the output.

Tests 1, 2, and 4 (test_get_command_strips_run_flags_and_stage_marker, test_get_command_normalizes_line_folds, test_get_command_preserves_quoted_arguments) will fail for these reasons. The fix is to avoid re-encoding: locate the index in the original value string where non-flag content begins, and slice value directly rather than reconstructing via shlex.join.

Prompt To Fix With AI
This is a comment left during a code review.
Path: press/press/doctype/deploy_candidate/docker_output_parsers.py
Line: 228-246

Comment:
**`shlex.join` re-encodes the command, breaking downstream splitting**

`_get_run_command` calls `shlex.split` (which strips quotes and consumes backslash-newline continuations) and then `shlex.join` (which re-quotes tokens with single quotes). This means: (1) `` `#stage-xxx` `` markers get re-quoted as `'`#stage-xxx`'`, so `get_command`'s `split("`#stage-")` no longer matches them correctly; (2) backslash-newline line folds are consumed by shlex, so `get_command`'s ` \\\n` split no longer works; (3) `"hello world"` (double-quoted) becomes `'hello world'` (single-quoted) in the output.

Tests 1, 2, and 4 (`test_get_command_strips_run_flags_and_stage_marker`, `test_get_command_normalizes_line_folds`, `test_get_command_preserves_quoted_arguments`) will fail for these reasons. The fix is to avoid re-encoding: locate the index in the original `value` string where non-flag content begins, and slice `value` directly rather than reconstructing via `shlex.join`.

How can I resolve this? If you propose a fix, please make it concise.



class StepMixin:
def _get_agent_step(
self,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
from unittest import TestCase

from press.press.doctype.deploy_candidate.docker_output_parsers import get_command


class TestDockerOutputParsers(TestCase):
def test_get_command_strips_run_flags_and_stage_marker(self):
name = "RUN --mount=type=cache,target=/root/.cache pip install -e apps/frappe `#stage-install-frappe`"

self.assertEqual(get_command(name), "pip install -e apps/frappe")

def test_get_command_normalizes_line_folds(self):
name = "RUN apt-get update \\\n && apt-get install -y curl \\\n && rm -rf /var/lib/apt/lists/* `#stage-setup-apt`"

self.assertEqual(
get_command(name),
"apt-get update\n&& apt-get install -y curl\n&& rm -rf /var/lib/apt/lists/*",
)

def test_get_command_keeps_json_form_run(self):
name = 'RUN ["python", "-m", "compileall", "apps"] `#stage-compile-assets`'

self.assertEqual(get_command(name), '["python", "-m", "compileall", "apps"]')

def test_get_command_preserves_quoted_arguments(self):
name = 'RUN echo "hello world" `#stage-print-message`'

self.assertEqual(get_command(name), 'echo "hello world"')
13 changes: 5 additions & 8 deletions press/press/doctype/log_counter/log_counter.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,11 @@
"Error Log": "method",
}

Counts = TypedDict(
"Counts",
{
"counts": dict[str, int],
"date": datetime.date,
"total": int,
},
)

class Counts(TypedDict):
counts: dict[str, int]
date: datetime.date
total: int


class LogCounter(Document):
Expand Down
44 changes: 34 additions & 10 deletions press/telegram_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@
# For license information, please see license.txt


import asyncio

import frappe
import telegram
from telegram.constants import MessageLimit, ParseMode

from press.utils import log_error

Expand All @@ -30,14 +33,13 @@ def send(self, message, html=False, reraise=False):
if not message:
return None
try:
text = message[: telegram.MAX_MESSAGE_LENGTH]
text = message[: MessageLimit.MAX_TEXT_LENGTH]
parse_mode = self._get_parse_mode(html)
return self.bot.send_message(
chat_id=self.chat_id,
text=text,
parse_mode=parse_mode,
message_thread_id=self.topic_id,
timeout=3,
return asyncio.run(
self._send_message(
text=text,
parse_mode=parse_mode,
)
)
except Exception:
if reraise:
Expand All @@ -52,8 +54,30 @@ def send(self, message, html=False, reraise=False):

def _get_parse_mode(self, html):
if html:
return telegram.ParseMode.HTML
return telegram.ParseMode.MARKDOWN
return ParseMode.HTML
return ParseMode.MARKDOWN

async def _send_message(self, text, parse_mode):
bot = self.bot
async with bot:
return await bot.send_message(
chat_id=self.chat_id,
text=text,
parse_mode=parse_mode,
message_thread_id=self.topic_id,
read_timeout=3,
write_timeout=3,
connect_timeout=3,
pool_timeout=3,
)

def _get_bot_username(self):
return asyncio.run(self._get_bot_username_async())

async def _get_bot_username_async(self):
bot = self.bot
async with bot:
return bot.username
Comment on lines +74 to +80
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 _get_bot_username() makes a live API call on every respond() invocation

respond() now calls _get_bot_username() for every incoming Telegram message, which creates a fresh Bot instance, calls asyncio.run(), and performs a full HTTP round-trip to Telegram just to obtain the bot's username — a value that never changes at runtime. In the old v13 API this was effectively free. A simple fix is to cache the username as an instance attribute on first use (e.g., self._username) so only the first message incurs the network call.

Prompt To Fix With AI
This is a comment left during a code review.
Path: press/telegram_utils.py
Line: 74-80

Comment:
**`_get_bot_username()` makes a live API call on every `respond()` invocation**

`respond()` now calls `_get_bot_username()` for every incoming Telegram message, which creates a fresh `Bot` instance, calls `asyncio.run()`, and performs a full HTTP round-trip to Telegram just to obtain the bot's username — a value that never changes at runtime. In the old v13 API this was effectively free. A simple fix is to cache the username as an instance attribute on first use (e.g., `self._username`) so only the first message incurs the network call.

How can I resolve this? If you propose a fix, please make it concise.


@property
def bot(self):
Expand Down Expand Up @@ -82,7 +106,7 @@ def respond(self, message):

mention = text[begin:end]
# Only respond to messages mentioning the bot
if mention != f"@{self.bot.username}":
if mention != f"@{self._get_bot_username()}":
return

command = text.replace(mention, "")
Expand Down
10 changes: 5 additions & 5 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,37 +22,37 @@ classifiers = [
"Programming Language :: Python :: 3.12",
]
dependencies = [
"ansible==3.4.0",
"ansible==13.6.0",
"beautifulsoup4",
"boto3==1.39.14",
"Click",
"coverage",
"dnspython==1.16.0",
"docker==6.1.2",
"dockerfile==3.2.0",
"oci==2.116.0",
"paramiko==3.3.1",
"pexpect==4.8.0",
"posthog==3.0.1",
"prometheus-client==0.19.0",
"PyGithub==1.44.1",
"pyOpenSSL~=23.2.0",
"python-telegram-bot==13.15",
"python-telegram-bot==22.7",
"razorpay>=1.3.0",
"requests<2.32",
"responses==0.23.1",
"selenium==4.23.1",
"semantic-version==2.10.0",
"sql_metadata==2.10.0",
"stripe~=2.56.0",
"stripe==15.1.0",
"tldextract==3.4.4",
"tomli==2.0.1",
"tqdm==4.66.3",
"twilio==8.10.3",
"wrapt~=1.15.0",
"elasticsearch-dsl>=8.0.0,<9.0.0",
"hcloud==2.2.1",
"playwright==1.49.1",
"greenlet==3.5.0",
"playwright==1.59.0",
"prometheus-api-client==0.6.0",
"pydo==0.24.0",
"semgrep==1.159.0",
Expand Down