Skip to content

UN-2190 [FEAT] Auto-capture execution ID in the API deployment Postman collection #2031

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

Open
athul-rs wants to merge 3 commits into
base: main
Choose a base branch
from
Open

UN-2190 [FEAT] Auto-capture execution ID in the API deployment Postman collection #2031

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0735fc8
UN-2190 Auto-capture execution_id in API deployment Postman collection
athul-rs Jun 11, 2026
fe248a4
UN-2190 Address review: guard non-JSON responses, scope variable to A…
athul-rs Jun 11, 2026
4bafbf7
UN-2190 Expose execution_id in the API execution response
athul-rs Jun 11, 2026
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
2 changes: 2 additions & 0 deletions backend/api_v2/postman_collection/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,6 @@ class CollectionKey:
EXECUTE_PIPELINE_API_KEY = "Process pipeline"
STATUS_API_KEY = "Execution status"
STATUS_EXEC_ID_DEFAULT = "REPLACE_WITH_EXECUTION_ID"
EXEC_ID_VARIABLE_NAME = "execution_id"

@jaseemjaskp jaseemjaskp Jun 18, 2026

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.

P3 — unenforced coupling between the variable name and its {{...}} reference. EXEC_ID_VARIABLE_NAME = "execution_id" and STATUS_EXEC_ID_VARIABLE = "{{execution_id}}" must agree (the status URL references {{execution_id}}, the capture JS sets "execution_id", and the collection variable keys on it). If one is renamed and the other isn't, the collection breaks silently — the status request would query a never-set variable. Derive one from the other so they can't drift:

STATUS_EXEC_ID_VARIABLE = f"{{{{{EXEC_ID_VARIABLE_NAME}}}}}"  # -> "{{execution_id}}"

STATUS_EXEC_ID_VARIABLE = "{{execution_id}}"
AUTH_QUERY_PARAM_DEFAULT = "REPLACE_WITH_API_KEY"
72 changes: 69 additions & 3 deletions backend/api_v2/postman_collection/dto.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,24 @@ class HeaderItem:
value: str


@dataclass
class ScriptItem:
exec: list[str]
type: str = "text/javascript"


@dataclass
class EventItem:
listen: str

@jaseemjaskp jaseemjaskp Jun 18, 2026

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.

P2 (type safety) — constrain the closed enums. EventItem.listen accepts any str, but Postman v2.1 only recognizes two hooks; a typo like "tests" produces a silently-ignored event with no error anywhere in this pipeline. Narrow it to Literal["prerequest", "test"]. Same idea for ScriptItem.type (line 24): only "text/javascript" is meaningful, so Literal["text/javascript"] (or keep the default and type it as such). Zero runtime cost, caught by mypy, self-documenting.

script: ScriptItem


@dataclass
class VariableItem:
key: str
value: str


@dataclass
class FormDataItem:
key: str
Expand Down Expand Up @@ -55,6 +73,7 @@ class RequestItem:
class PostmanItem:
name: str
request: RequestItem
event: list[EventItem] | None = None

@jaseemjaskp jaseemjaskp Jun 18, 2026

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.

P2 — event: list[EventItem] | None = None creates a None/[] tri-state where only "some events" / "no events" are meaningful, and it's the sole reason to_dict() (lines 301-306) has to post-process and pop null event keys. Other "maybe-empty" fields on these DTOs already use field(default_factory=list) (PostmanCollection.item/variable, lines 251-252). Aligning here:

event: list[EventItem] = field(default_factory=list)

lets you drop the strip loop and reduce to_dict to return asdict(self). Trade-off: items then serialize "event": [] instead of omitting the key — both are valid Postman v2.1 (empty array is the schema-canonical form). If preserving the exact "key absent" output is a hard requirement, keep the loop but make it robust to the new default by testing if not item.get("event") rather than is None.



@dataclass
Expand All @@ -69,6 +88,12 @@ class APIBase(ABC):
def get_form_data_items(self) -> list[FormDataItem]:
pass

def get_collection_variables(self) -> list["VariableItem"]:

@jaseemjaskp jaseemjaskp Jun 18, 2026

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.

P3 (nit) — unnecessary string forward-reference. VariableItem is already defined above (line 33), so the quotes in -> list["VariableItem"] aren't needed; the override at line 175 already uses the bare list[VariableItem]. Use the bare type for consistency.

"""Collection-level variables; only needed when a request
references them.
"""
return []

@abstractmethod
def get_api_endpoint(self) -> str:
pass
Expand Down Expand Up @@ -137,20 +162,52 @@ def get_api_endpoint(self) -> str:
def _get_status_api_request(self) -> RequestItem:
header_list = [HeaderItem(key="Authorization", value=f"Bearer {self.api_key}")]
status_query_param = {
"execution_id": CollectionKey.STATUS_EXEC_ID_DEFAULT,
"execution_id": CollectionKey.STATUS_EXEC_ID_VARIABLE,
ApiExecution.INCLUDE_METADATA: "False",
ApiExecution.INCLUDE_METRICS: "False",
}
status_query_str = urlencode(status_query_param)
# Keep {{...}} unescaped so Postman resolves the collection variable
status_query_str = urlencode(status_query_param, safe="{}")
abs_api_endpoint = urljoin(settings.WEB_APP_ORIGIN_URL, self.api_endpoint)
status_url = urljoin(abs_api_endpoint, "?" + status_query_str)
return RequestItem(method=HTTPMethod.GET, header=header_list, url=status_url)

def get_collection_variables(self) -> list[VariableItem]:
return [
VariableItem(
key=CollectionKey.EXEC_ID_VARIABLE_NAME,
value=CollectionKey.STATUS_EXEC_ID_DEFAULT,
)
]

def _get_execute_capture_event(self) -> EventItem:
"""Post-response script that stores the execution_id from the execute

@jaseemjaskp jaseemjaskp Jun 18, 2026

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.

P3 (comment) — docstring omits the load-bearing response-shape coupling. The script silently no-ops unless the response is {message: {execution_id}}; if that envelope ever changes (e.g. message renamed or execution_id relocated) capture breaks quietly. Add a line so a future API change flags this script, e.g. "Assumes the execute response has shape {message: {execution_id}}; if the body is non-JSON or lacks that field, nothing is captured." Minor: "Post-response script" matches Postman's current UI label but the code emits listen="test" — a half-sentence tying the two ("post-response, i.e. Postman's test hook") avoids a maintainer double-take.

response into a collection variable, so the status request can use it
without manual copy-pasting.
"""
return EventItem(
listen="test",
script=ScriptItem(
exec=[
"let response = null;",
"try {",
" response = pm.response.json();",
"} catch (error) {",
" // Non-JSON response (e.g. gateway error); nothing to capture",
"}",
"if (response && response.message && response.message.execution_id) {", # noqa: E501

@jaseemjaskp jaseemjaskp Jun 18, 2026

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 — silent reuse of a stale execution_id. This guard has no else branch, and it's distinct from the non-JSON try/catch already fixed below: here the response parses fine but carries no message.execution_id — e.g. a 422 validation error or 500 ({"message": {"error": ...}}), auth/rate-limit rejection, or any 2xx with an unexpected shape. In that case the script does nothing and {{execution_id}} keeps whatever it held — most dangerously the id captured from a previous successful run. The user re-runs Execute, it fails, hits Execution status, and Postman silently polls the old execution and shows its COMPLETED result as if it belonged to the failed run. No signal that capture was skipped.

Suggest making the miss explicit and preventing cross-run staleness:

if (response && response.message && response.message.execution_id) {
    pm.collectionVariables.set("execution_id", response.message.execution_id);
} else {
    pm.collectionVariables.set("execution_id", "REPLACE_WITH_EXECUTION_ID");
    console.warn("No execution_id in execute response; not reusing a stale value. Status:", pm.response.code);
}

Optionally gate capture on pm.response.code 2xx so a 200-with-unexpected-shape is surfaced too.

f' pm.collectionVariables.set("{CollectionKey.EXEC_ID_VARIABLE_NAME}", response.message.execution_id);', # noqa: E501
Comment thread
greptile-apps[bot] marked this conversation as resolved.
"}",
]
),
Comment thread
greptile-apps[bot] marked this conversation as resolved.
)

def get_postman_items(self) -> list[PostmanItem]:
postman_item_list = [
PostmanItem(
name=CollectionKey.EXECUTE_API_KEY,
request=self.get_create_api_request(),
event=[self._get_execute_capture_event()],
),
PostmanItem(
name=CollectionKey.STATUS_API_KEY,
Expand Down Expand Up @@ -192,6 +249,7 @@ def get_postman_items(self) -> list[PostmanItem]:
class PostmanCollection:
info: PostmanInfo
item: list[PostmanItem] = field(default_factory=list)
variable: list[VariableItem] = field(default_factory=list)

@classmethod
def create(
Expand Down Expand Up @@ -228,7 +286,11 @@ def create(
)
postman_info: PostmanInfo = data_object.get_postman_info()
postman_item_list = data_object.get_postman_items()
return cls(info=postman_info, item=postman_item_list)
return cls(
info=postman_info,
item=postman_item_list,
variable=data_object.get_collection_variables(),
)

def to_dict(self) -> dict[str, Any]:
"""Convert PostmanCollection instance to a dict.
Expand All @@ -237,4 +299,8 @@ def to_dict(self) -> dict[str, Any]:
dict[str, Any]: PostmanCollection as a dict
"""
collection_dict = asdict(self)
# Drop null event blocks; Postman expects "event" to be a list

@jaseemjaskp jaseemjaskp Jun 18, 2026

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.

P2 (test coverage) — backend/api_v2/postman_collection/ has no tests at all, and this PR's trickiest logic is exactly the kind that fails only at Postman-import time, not in CI. to_dict() (and create()) are pure given settings.WEB_APP_ORIGIN_URL + an api_key, so this is cheap to lock down. Suggest adding backend/api_v2/postman_collection/tests/test_dto.py covering: (a) to_dict strips null event yet keeps the populated execute event and emits the variable array; (b) regression guard that PipelineDto collections carry no event key and variable == [] while APIDeploymentDto carries both; (c) the variable key, the {{execution_id}} in the status URL, and the pm.collectionVariables.set(...) in the JS all reference the same CollectionKey.EXEC_ID_VARIABLE_NAME (assert via the constant, not a hardcoded literal, so a partial rename fails); (d) the status URL keeps execution_id={{execution_id}} literal (no %7B). Assert load-bearing substrings, not the whole exec array, to stay robust to comment/wording edits.

for item in collection_dict.get("item", []):
if item.get("event") is None:
item.pop("event", None)
return collection_dict
1 change: 1 addition & 0 deletions backend/api_v2/serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,7 @@ class DeploymentResponseSerializer(Serializer):


class APIExecutionResponseSerializer(Serializer):
execution_id = CharField()
execution_status = CharField()
status_api = CharField()
error = CharField()
Expand Down
Loading