Conversation
There was a problem hiding this comment.
Pull request overview
Adds conditional logic support (!Conditional) to Prismarine table configuration/triggers and to imported easysam.yaml files, with example updates for AOSS conditional enablement.
Changes:
- Resolve
!Conditionalblocks in rootresources.yamland importedeasysam.yamlfiles before validation/preprocessing. - Add Prismarine support for conditionally included tables (
conditional-tables) and for conditional trigger configuration. - Update examples (new
prismarineconditionals, and conditional gating inexample/aoss) and adjust AOSS test expectations.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_aoss.py | Extends AOSS generation test to assert failure in one env and success in devaoss. |
| src/easysam/schemas.json | Extends Prismarine schema with conditional-tables and allows empty trigger objects post-resolution. |
| src/easysam/load.py | Threads deploy_ctx through preprocessors; resolves conditionals in imports; adds conditional-tables merging + trigger stripping logic; expands resolve_conditionals recursion to lists. |
| example/prismarineconditionals/thirdparty/requirements.txt | Adds third-party deps for the new Prismarine conditionals example. |
| example/prismarineconditionals/resources.yaml | Demonstrates conditional triggers and conditional Prismarine table inclusion. |
| example/prismarineconditionals/common/utils.py | Example utility for environment selection. |
| example/prismarineconditionals/common/myobject/models.py | Example Prismarine model with a trigger. |
| example/prismarineconditionals/common/myobject/db.py | Example DB access wrapper around generated Prismarine client. |
| example/prismarineconditionals/common/myobject/advanced_example.py | Expanded example showing multiple trigger configurations. |
| example/prismarineconditionals/common/dynamo_access.py | Example custom DynamoAccess implementation. |
| example/prismarineconditionals/common/condition/models.py | Example conditional model/table definition. |
| example/prismarineconditionals/common/condition/db.py | Example DB wrapper for conditional model. |
| example/prismarineconditionals/backend/function/itemlogger/index.py | Example Lambda handler used as a DynamoDB stream trigger. |
| example/prismarineconditionals/backend/function/itemlogger/easysam.yaml | Example import file defining the trigger Lambda. |
| example/prismarineconditionals/README.md | Documentation for the new Prismarine conditional triggers/tables example. |
| example/prismarineconditionals/.gitignore | Ignores build artifacts and generated files for the new example. |
| example/aoss/resources.yaml | Wraps search configuration in a !Conditional block. |
| example/aoss/backend/function/searchfunc/easysam.yaml | Conditionally includes the lambda definition for AOSS-related envs. |
| example/aoss/backend/function/indexfunc/easysam.yaml | Conditionally includes the lambda definition for AOSS-related envs. |
| example/aoss/backend/database/easysam.yaml | Conditionally includes table definitions for AOSS-related envs. |
| example/README.md | Adds a reference to the new prismarineconditionals example. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| tables = prismarine_dynamo_tables( | ||
| prefix, base, package, resources_dir, pypath, errors | ||
| ) | ||
|
|
||
| for table_name, table in tables.items(): | ||
| trigger = table.get('trigger') |
There was a problem hiding this comment.
tables can be None when prismarine_dynamo_tables(...) fails, but the code iterates tables.items() before checking if not tables. This will raise an AttributeError and prevent graceful error accumulation. Move the if not tables: ... continue guard to immediately after the call, before any iteration/use of tables.
| f'Removing trigger {trigger}' | ||
| f'from table {table_name}' |
There was a problem hiding this comment.
This log message concatenates two f-strings without a separating space, producing output like Removing trigger <...>from table <...>. Add a space (or combine into one f-string) to keep logs readable.
| f'Removing trigger {trigger}' | |
| f'from table {table_name}' | |
| f'Removing trigger {trigger} from table {table_name}' |
| def resolve_conditionals(resources_data, deploy_ctx: dict[str, str], errors: list[str]): | ||
|
|
||
| if isinstance(resources_data, list): | ||
| return [ | ||
| resolve_conditionals(item, deploy_ctx, errors) | ||
| for item in resources_data | ||
| ] |
There was a problem hiding this comment.
New conditional-resolution behavior was added for lists and imported files, but there are no targeted tests asserting list handling and conditional inclusion/exclusion in imported easysam.yaml files. Add unit tests to cover (1) resolve_conditionals with list values and (2) conditional blocks inside imported files to prevent regressions.
| ```bash | ||
| easysam --environment dev inspect schema example/prismarineconditional | ||
| easysam --environment dev generate example/prismarineconditional | ||
| ``` |
There was a problem hiding this comment.
The example commands reference example/prismarineconditional, but the directory added in this PR is example/prismarineconditionals (with an “s”). As written, the commands will fail when copy/pasted.
| - No stream, no Lambda, no logs | ||
|
|
||
| - **Conditional table (`condition`)** | ||
| - Created only in allowed environments (`devaoss`, `prodsam`, `stagingsam`) |
There was a problem hiding this comment.
The README claims the conditional table is created in devaoss, prodsam, and stagingsam, but resources.yaml for this example only lists prodsam and stagingsam for conditional-tables. Update either the README or the config so they match.
| - Created only in allowed environments (`devaoss`, `prodsam`, `stagingsam`) | |
| - Created only in allowed environments (`prodsam`, `stagingsam`) |
| import common.myobject.prismarine_client as pc | ||
|
|
||
|
|
||
| class ConditionModel(pc.ItemModel): |
There was a problem hiding this comment.
This example module appears to be for the condition package, but it imports common.myobject.prismarine_client and subclasses pc.ItemModel. This will point to the wrong generated client/model. Import the condition client module and subclass the model class corresponding to the Condition table instead.
| import common.myobject.prismarine_client as pc | |
| class ConditionModel(pc.ItemModel): | |
| import common.condition.prismarine_client as pc | |
| class ConditionModel(pc.ConditionModel): |
Adds support for conditional logic in Prismarine tables and their triggers, as well as handling of conditions in imported files. Introduces optimizations for AOSS as part of these changes.