Skip to content
Merged
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
39 changes: 39 additions & 0 deletions rust-bindings/src/expr/symbol_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,45 @@ impl PySerializedSymbolTable {
}
}

/// Build a ``SerializedSymbolTable`` from its JSON transport
/// text, as produced by ``to_json_str``.
///
/// This is the inverse of ``to_json_str`` and exists for callers
/// that carry the transport form across a process or service
/// boundary — a scheduler persisting the table produced by
/// ``create_job`` and later handing it to a worker, for instance.
/// Prefer ``from_symtab`` when the source is an in-memory
/// ``SymbolTable``.
///
/// Raises ``ValueError`` if the text is not valid JSON. Note that
/// the *contents* are validated lazily: a well-formed JSON
/// document whose entries are not valid symbol table entries is
/// accepted here and rejected by ``to_symtab``.
#[classmethod]
fn from_json_str(_cls: &Bound<'_, pyo3::types::PyType>, json: &str) -> PyResult<Self> {
let inner = openjd_expr::SerializedSymbolTable::from_json_str(json).map_err(|e| {
pyo3::exceptions::PyValueError::new_err(format!(
"Failed to parse SerializedSymbolTable JSON: {e}"
))
})?;
Ok(Self { inner })
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Design note on the lazy-validation choice: for the use case the docstring names — a scheduler persisting the transport text and later handing it to a worker — deferring content validation to to_symtab pushes the error to the worst possible place. The scheduler happily accepts and stores a bad payload, and the failure surfaces later on the worker, far from the input that caused it.

Validating structure at ingest (i.e. parsing into the real entry type rather than a loose JSON document) would make from_json_str a real trust boundary and give the error at the point where the caller still has the offending text in hand. If the laziness is deliberate — e.g. openjd_expr::SerializedSymbolTable::from_json_str intentionally keeps the raw document so unknown future entry kinds pass through untouched — it would help to say so in the docstring, since as written it reads as an accident rather than forward-compatibility.


/// Serialize to the JSON transport text: an array of
/// ``{"name", "type", "value"}`` objects in canonical
/// (lexicographic) path order.
///
/// Use this to move a table across a process or service boundary;
/// pair it with ``from_json_str`` to reconstruct. The result is
/// stable for a given table, so it is safe to store or compare.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The two guarantees documented here — an array of name/type/value objects "in canonical (lexicographic) path order", and "stable for a given table, so it is safe to store or compare" — only hold for instances built via from_symtab.

from_json_str is explicitly lazy (per its own docstring, and confirmed by test_from_json_str_defers_content_validation_to_to_symtab, which stores a non-array document successfully). So inner can hold arbitrary well-formed JSON, and to_json_str re-emits it as-is. Consequences:

  • to_json_str() on a from_json_str-derived instance is not necessarily an array, and not necessarily in canonical path order.
  • Byte-comparing two to_json_str() outputs is not equivalent to comparing table contents: a hand-built transport string with entries in non-lexicographic order, or with duplicate names, survives the round trip and compares unequal to the from_symtab form of the same table.

Since the docstring actively invites callers to store and compare the text, consider scoping the claim (e.g. canonical order and byte-stability hold for tables built via from_symtab; from_json_str preserves the input text as given), or normalizing on ingest in from_json_str so the invariant is unconditional.

fn to_json_str(&self) -> PyResult<String> {
serde_json::to_string(&self.inner).map_err(|e| {
pyo3::exceptions::PyValueError::new_err(format!(
"Failed to serialize SerializedSymbolTable: {e}"
))
})
}

/// Deserialize this serialized symbol table into a full
/// ``SymbolTable`` suitable for inspection or modification.
/// ``path_format`` controls how PATH-typed values are
Expand Down
30 changes: 30 additions & 0 deletions src/openjd/_openjd_rs.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -2032,6 +2032,36 @@ class SerializedSymbolTable:
use this classmethod to convert.
"""

@classmethod
def from_json_str(cls, json: builtins.str) -> SerializedSymbolTable:
r"""
Build a ``SerializedSymbolTable`` from its JSON transport
text, as produced by ``to_json_str``.

This is the inverse of ``to_json_str`` and exists for callers
that carry the transport form across a process or service
boundary — a scheduler persisting the table produced by
``create_job`` and later handing it to a worker, for instance.
Prefer ``from_symtab`` when the source is an in-memory
``SymbolTable``.

Raises ``ValueError`` if the text is not valid JSON. Note that
the *contents* are validated lazily: a well-formed JSON
document whose entries are not valid symbol table entries is
accepted here and rejected by ``to_symtab``.
"""

def to_json_str(self) -> builtins.str:
r"""
Serialize to the JSON transport text: an array of
``{"name", "type", "value"}`` objects in canonical
(lexicographic) path order.

Use this to move a table across a process or service boundary;
pair it with ``from_json_str`` to reconstruct. The result is
stable for a given table, so it is safe to store or compare.
"""

def to_symtab(self, *, path_format: typing.Optional[PathFormat] = None) -> SymbolTable:
r"""
Deserialize this serialized symbol table into a full
Expand Down
9 changes: 8 additions & 1 deletion src/openjd/model/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.

from ._capabilities import validate_attribute_capability_name, validate_amount_capability_name
from ._create_job import create_job, preprocess_job_parameters
from ._create_job import (
JobWithSymbolTables,
create_job,
create_job_with_symbol_tables,
preprocess_job_parameters,
)
from ._errors import (
CompatibilityError,
DecodeValidationError,
Expand Down Expand Up @@ -51,6 +56,8 @@

__all__ = (
"create_job",
"create_job_with_symbol_tables",
"JobWithSymbolTables",
"decode_template",
"decode_environment_template",
"decode_job_template",
Expand Down
149 changes: 143 additions & 6 deletions src/openjd/model/_create_job.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,22 @@
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.

import json
from dataclasses import dataclass
from os.path import normpath
from pathlib import Path
from typing import Any, Optional, cast
from typing import TYPE_CHECKING, Any, Optional, cast

from pydantic import ValidationError

from ._errors import CompatibilityError, DecodeValidationError
from ._format_strings import FormatStringError
from ._symbol_table import SymbolTable

if TYPE_CHECKING:
# Type-only: importing openjd.expr loads the Rust bindings, and importing this
# module must not. The runtime imports live in _serialize_symbol_table, which
# only the opt-in create_job_with_symbol_tables path reaches.
from openjd.expr import SerializedSymbolTable
from ._internal import instantiate_model
from ._merge_job_parameter import merge_job_parameter_definitions
from ._types import (
Expand All @@ -26,7 +33,28 @@
)
from ._convert_pydantic_error import pydantic_validationerrors_to_str

__all__ = ("preprocess_job_parameters",)
__all__ = ("preprocess_job_parameters", "create_job_with_symbol_tables", "JobWithSymbolTables")


@dataclass(frozen=True)
class JobWithSymbolTables:
"""A created job together with the symbol tables it was instantiated with.

Returned by :func:`create_job_with_symbol_tables`. The tables are in the
``SerializedSymbolTable`` transport form, ready to persist or send to the host
that will run the job's sessions.
"""

job: Job
"""The created job — identical to what ``create_job`` returns."""

job_symbol_table: "SerializedSymbolTable"
"""Job scope: ``Param.*``, ``RawParam.*``, and ``Job.Name`` under EXPR."""

step_symbol_tables: dict[str, "SerializedSymbolTable"]
"""Step scope keyed by step name: job scope plus ``Step.Name`` and the step's
evaluated template-scope ``let`` bindings."""


# The original scalar job-parameter type names whose values are carried as
# strings through preprocessing. EXPR-extension types (BOOL, RANGE_EXPR, and
Expand Down Expand Up @@ -386,12 +414,12 @@ def preprocess_job_parameters(
# =======================================================================


def create_job(
def _create_job_and_symbol_table(
*,
job_template: JobTemplate,
job_parameter_values: JobParameterValues,
environment_templates: Optional[list[EnvironmentTemplate]] = None,
) -> Job:
) -> tuple[Job, SymbolTable]:
"""This function will create a job from a given Job Template and set of values for
Job Parameters. Minimally, values must be provided for Job Parameters that do not have
default values defined in the template.
Expand All @@ -409,7 +437,8 @@ def create_job(
DecodeValidationError

Returns:
Job: The job generated.
tuple[Job, SymbolTable]: The job generated, and the job-scope symbol table
it was instantiated with.
"""

# Raises: ValueError
Expand Down Expand Up @@ -489,4 +518,112 @@ def create_job(
pydantic_validationerrors_to_str(job_template.__class__, exc.errors())
)

return cast(Job, job)
return cast(Job, job), symtab


def create_job(
*,
job_template: JobTemplate,
job_parameter_values: JobParameterValues,
environment_templates: Optional[list[EnvironmentTemplate]] = None,
) -> Job:
"""Create a job from a Job Template and a set of Job Parameter values.

See :func:`create_job_with_symbol_tables` when you also need the resolved
symbol tables — for instance to transport them to a host that will run the
job's sessions.

Raises:
DecodeValidationError

Returns:
Job: The job generated.
"""
job, _symtab = _create_job_and_symbol_table(
job_template=job_template,
job_parameter_values=job_parameter_values,
environment_templates=environment_templates,
)
return job


def create_job_with_symbol_tables(
*,
job_template: JobTemplate,
job_parameter_values: JobParameterValues,
environment_templates: Optional[list[EnvironmentTemplate]] = None,
) -> JobWithSymbolTables:
"""Create a job, and return the resolved symbol tables alongside it.

Same job as :func:`create_job`. The difference is that the symbol tables
built during instantiation are returned instead of discarded, in the
``SerializedSymbolTable`` transport form, so a caller can persist them and
hand them to a session on another host.

This mirrors ``openjd-rs``, whose ``create_job`` attaches the step-scope
table to each ``Step`` as ``resolved_symtab``. The tables are returned
separately here rather than added to the ``Job`` model so that the model's
serialized form does not change.

Scopes, matching the sessions API:

* ``job_symbol_table`` — job scope: ``Param.*``, ``RawParam.*`` and, with the
EXPR extension, ``Job.Name``. Use it for job and queue environments.
* ``step_symbol_tables`` — job scope plus ``Step.Name`` and the step's
evaluated template-scope ``let`` bindings, keyed by step name. Use each for
that step and its step environments.

Script-scope ``let`` bindings are deliberately absent: they resolve at
session time, so the table carries the symbols they reference rather than
their results.

Unlike ``openjd-rs``, the tables are not filtered down to the symbols a step
actually references. They are a superset, which is valid input wherever a
filtered table is — a session layers its own scopes on top either way.

Raises:
DecodeValidationError

Returns:
JobWithSymbolTables: The job and its resolved symbol tables.
"""
job, symtab = _create_job_and_symbol_table(
job_template=job_template,
job_parameter_values=job_parameter_values,
environment_templates=environment_templates,
)

step_tables: dict[str, SerializedSymbolTable] = {}
for step_template in getattr(job_template, "steps", None) or []:
# Reuse the model's own per-step hook — the one instantiate_model calls —
# so the returned table is the step scope the job was instantiated with
# rather than a reimplementation of it. It depends only on the step's
# name, its `let` bindings and the job-scope table, so re-invoking it
# here is deterministic.
extends_symtab = step_template._job_creation_metadata.extends_symtab
step_symtab = extends_symtab(step_template, symtab) if extends_symtab else symtab
step_tables[str(step_template.name)] = _serialize_symbol_table(step_symtab)

return JobWithSymbolTables(
job=job,
job_symbol_table=_serialize_symbol_table(symtab),
step_symbol_tables=step_tables,
)


def _serialize_symbol_table(symtab: SymbolTable) -> "SerializedSymbolTable":
"""Convert a job-model symbol table into the EXPR transport form.

Goes through ``symtab_to_expr_values`` so the typed coercion is the engine's
own, then through ``SerializedSymbolTable.from_symtab`` — the same serializer
openjd-rs uses — so the bytes match what the Rust implementation produces for
an equivalent table.
"""
# Imported here rather than at module scope: both of these load the Rust
# bindings, and importing openjd.model must not.
from openjd.expr import SerializedSymbolTable

from ._format_strings._expr_support import symtab_to_expr_values

engine_symtab = symtab_to_expr_values(symtab, types=symtab.expr_types)
return SerializedSymbolTable.from_symtab(engine_symtab)
Loading
Loading