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

DM-47529: Migrate Prompt Processing PipelinesConfig to YAML #222

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

kfindeisen
Copy link
Member

This PR replaces the bespoke pipelines config format with an extensible YAML-based format, and modifies the config semantics to behave like a prioritized list instead of a mapping. These changes will make it much easier to add new kinds of constraints to the format in the future.

A typical run has two simultaneous instances (one for preprocessing,
one for the main pipeline), so this disclaimer is redundant.
Delegating parsing to YAML makes it much easier to add more fields to
the pipelines spec, and even this basic implementation catches some
escaping errors in the original tests.
Making the activator responsible for YAML input leads to better
seperation of concerns. It also makes unit tests slightly
better-behaved, because they can be expressed in pure Python. However,
this change makes some test cases obsolete, because they specifically
test text formatting.
Previously, pipeline configs behaved as a mapping from survey to
pipeline list; this design didn't support more complex specs (wildcards,
spatial constraints, etc.). Taking the first block that "matches" a
visit allows for more free-form specification of when pipelines should
be run.
This change allows for cleaner separation of concerns, as all input
handling is now done by _Spec.
No longer requiring a survey makes it possible to naturally add in other
optional constraints, such as position.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants