-
Notifications
You must be signed in to change notification settings - Fork 0
DM-37253: Make Prompt Processing service configurable #71
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
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
025ac28
Fix deprecation warnings for ExposureIdInfo.
kfindeisen f95b70a
Add PipelinesConfig class.
kfindeisen 7de0ac5
Implement PipelinesConfig.get_pipeline_file.
kfindeisen 2e7e790
Use PipelinesConfig in MiddlewareInterface.
kfindeisen b9dd9c8
Add simple config input to PipelinesConfing.
kfindeisen 16116dc
Do not assume ApPipe.yaml in error messages.
kfindeisen 3f950ca
Move test pipeline to tests/ directory.
kfindeisen 95608fa
Support empty survey strings in PipelinesConfig.
kfindeisen 2f33a6c
Add PipelinesConfig unit test for multiline configs.
kfindeisen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,137 @@ | ||
# This file is part of prompt_prototype. | ||
# | ||
# Developed for the LSST Data Management System. | ||
# This product includes software developed by the LSST Project | ||
# (https://www.lsst.org). | ||
# See the COPYRIGHT file at the top-level directory of this distribution | ||
# for details of code ownership. | ||
# | ||
# This program is free software: you can redistribute it and/or modify | ||
# it under the terms of the GNU General Public License as published by | ||
# the Free Software Foundation, either version 3 of the License, or | ||
# (at your option) any later version. | ||
# | ||
# This program is distributed in the hope that it will be useful, | ||
# but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
# GNU General Public License for more details. | ||
# | ||
# You should have received a copy of the GNU General Public License | ||
# along with this program. If not, see <https://www.gnu.org/licenses/>. | ||
|
||
|
||
__all__ = ["PipelinesConfig"] | ||
|
||
|
||
import collections.abc | ||
import os | ||
import re | ||
|
||
from .visit import FannedOutVisit | ||
|
||
|
||
class PipelinesConfig: | ||
"""A pipeline configuration for the Prompt Processing service. | ||
|
||
This class provides the execution framework with a simple interface for | ||
identifying the pipeline to execute. It attempts to abstract the details of | ||
which factors affect the choice of pipeline to make it easier to add new | ||
features in the future. | ||
|
||
Parameters | ||
---------- | ||
config : `str` | ||
A string describing pipeline selection criteria. The current format is | ||
a space-delimited list of mappings, each of which has the format | ||
``(survey="<survey>")=<pipeline>``. The pipeline path may contain | ||
environment variables. No key or value may contain the "=" character. | ||
See examples below. | ||
|
||
Notes | ||
----- | ||
While it is not expected that there will ever be more than one | ||
PipelinesConfig instance in a program's lifetime, this class is *not* a | ||
singleton and objects must be passed explicitly to the code that | ||
needs them. | ||
|
||
Examples | ||
-------- | ||
A single-survey config: | ||
|
||
>>> PipelinesConfig('(survey="TestSurvey")=/etc/pipelines/SingleFrame.yaml') # doctest: +ELLIPSIS | ||
<config.PipelinesConfig object at 0x...> | ||
|
||
A config with multiple surveys, and environment variables: | ||
|
||
>>> PipelinesConfig('(survey="TestSurvey")=/etc/pipelines/ApPipe.yaml ' | ||
... '(survey="Camera Test")=${AP_PIPE_DIR}/pipelines/LSSTComCam/Isr.yaml ' | ||
... '(survey="")=${AP_PIPE_DIR}/pipelines/LSSTComCam/Isr.yaml ') | ||
... # doctest: +ELLIPSIS | ||
<config.PipelinesConfig object at 0x...> | ||
""" | ||
|
||
def __init__(self, config: str): | ||
if not config: | ||
raise ValueError("Must configure at least one pipeline.") | ||
|
||
self._mapping = self._parse_config(config) | ||
|
||
@staticmethod | ||
def _parse_config(config: str) -> collections.abc.Mapping: | ||
"""Turn a config string into structured config information. | ||
|
||
Parameters | ||
---------- | ||
config : `str` | ||
A string describing pipeline selection criteria. The current format | ||
is a space-delimited list of mappings, each of which has the format | ||
'(survey="<survey>")=<pipeline>'. The pipeline path may contain | ||
environment variables. No key or value may contain the "=" | ||
character. | ||
|
||
Returns | ||
------- | ||
config : mapping [`str`, `str`] | ||
A mapping from the survey type to the pipeline to run for that | ||
survey. A more complex key or container type may be needed in the | ||
future, if other pipeline selection criteria are added. | ||
|
||
Raises | ||
------ | ||
ValueError | ||
Raised if the string cannot be parsed. | ||
""" | ||
# Use regex instead of str.split, in case keys or values also have spaces. | ||
node = re.compile(r'\s*\(survey="(?P<survey>[\w\s]*)"\)=' | ||
r'(?P<filename>[-\w./${} ]*[-\w./${}])(?:\s+|$)') | ||
|
||
items = {} | ||
pos = 0 | ||
match = node.match(config, pos) | ||
while match: | ||
items[match['survey']] = match['filename'] | ||
|
||
pos = match.end() | ||
match = node.match(config, pos) | ||
if pos != len(config): | ||
raise ValueError(f"Unexpected text at position {pos}: '{config[pos:]}'.") | ||
|
||
return items | ||
|
||
def get_pipeline_file(self, visit: FannedOutVisit) -> str: | ||
"""Identify the pipeline to be run, based on the provided visit. | ||
|
||
Parameters | ||
---------- | ||
visit : `activator.visit.FannedOutVisit` | ||
The visit for which a pipeline must be selected. | ||
|
||
Returns | ||
------- | ||
pipeline : `str` | ||
A path to a configured pipeline file. | ||
""" | ||
try: | ||
return os.path.expandvars(self._mapping[visit.survey]) | ||
except KeyError as e: | ||
raise RuntimeError(f"Unsupported survey: {visit.survey}") from e |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,161 @@ | ||
# This file is part of prompt_prototype. | ||
# | ||
# Developed for the LSST Data Management System. | ||
# This product includes software developed by the LSST Project | ||
# (https://www.lsst.org). | ||
# See the COPYRIGHT file at the top-level directory of this distribution | ||
# for details of code ownership. | ||
# | ||
# This program is free software: you can redistribute it and/or modify | ||
# it under the terms of the GNU General Public License as published by | ||
# the Free Software Foundation, either version 3 of the License, or | ||
# (at your option) any later version. | ||
# | ||
# This program is distributed in the hope that it will be useful, | ||
# but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
# GNU General Public License for more details. | ||
# | ||
# You should have received a copy of the GNU General Public License | ||
# along with this program. If not, see <https://www.gnu.org/licenses/>. | ||
|
||
|
||
import dataclasses | ||
import os | ||
import unittest | ||
|
||
from lsst.utils import getPackageDir | ||
|
||
from activator.config import PipelinesConfig | ||
from activator.visit import FannedOutVisit | ||
|
||
|
||
TESTDIR = os.path.abspath(os.path.dirname(__file__)) | ||
|
||
|
||
class PipelinesConfigTest(unittest.TestCase): | ||
def setUp(self): | ||
super().setUp() | ||
|
||
self.visit = FannedOutVisit( | ||
instrument="NotACam", | ||
detector=42, | ||
groupId="2023-01-23T23:33:14.762", | ||
nimages=2, | ||
filters="k2022", | ||
coordinateSystem=FannedOutVisit.CoordSys.ICRS, | ||
position=[134.5454, -65.3261], | ||
rotationSystem=FannedOutVisit.RotSys.SKY, | ||
cameraAngle=135.0, | ||
survey="TestSurvey", | ||
salIndex=42, | ||
scriptSalIndex=42, | ||
dome=FannedOutVisit.Dome.OPEN, | ||
duration=35.0, | ||
totalCheckpoints=1, | ||
) | ||
|
||
def test_main_survey(self): | ||
config = PipelinesConfig( | ||
' (survey="TestSurvey")=${PROMPT_PROTOTYPE_DIR}/pipelines/NotACam/ApPipe.yaml') | ||
self.assertEqual( | ||
config.get_pipeline_file(self.visit), | ||
os.path.normpath(os.path.join(TESTDIR, "..", "pipelines", "NotACam", "ApPipe.yaml")) | ||
) | ||
|
||
def test_selection(self): | ||
config = PipelinesConfig('(survey="TestSurvey")=/etc/pipelines/SingleFrame.yaml ' | ||
'(survey="CameraTest")=${AP_PIPE_DIR}/pipelines/Isr.yaml ' | ||
'(survey="")=Default.yaml ' | ||
) | ||
self.assertEqual( | ||
config.get_pipeline_file(self.visit), | ||
os.path.normpath(os.path.join("/etc", "pipelines", "SingleFrame.yaml")) | ||
) | ||
self.assertEqual( | ||
config.get_pipeline_file(dataclasses.replace(self.visit, survey="CameraTest")), | ||
os.path.normpath(os.path.join(getPackageDir("ap_pipe"), "pipelines", "Isr.yaml")) | ||
) | ||
self.assertEqual( | ||
config.get_pipeline_file(dataclasses.replace(self.visit, survey="")), | ||
"Default.yaml" | ||
) | ||
|
||
def test_multiline(self): | ||
config = PipelinesConfig('''(survey="TestSurvey")=/etc/pipelines/SingleFrame.yaml | ||
(survey="CameraTest")=${AP_PIPE_DIR}/pipelines/Isr.yaml | ||
''' | ||
) | ||
self.assertEqual( | ||
config.get_pipeline_file(self.visit), | ||
os.path.normpath(os.path.join("/etc", "pipelines", "SingleFrame.yaml")) | ||
) | ||
self.assertEqual( | ||
config.get_pipeline_file(dataclasses.replace(self.visit, survey="CameraTest")), | ||
os.path.normpath(os.path.join(getPackageDir("ap_pipe"), "pipelines", "Isr.yaml")) | ||
) | ||
|
||
def test_space(self): | ||
config = PipelinesConfig('(survey="TestSurvey")=/dir with space/pipelines/SingleFrame.yaml ' | ||
'(survey="Camera Test")=${AP_PIPE_DIR}/pipe lines/Isr.yaml ' | ||
) | ||
self.assertEqual( | ||
config.get_pipeline_file(self.visit), | ||
os.path.normpath(os.path.join("/dir with space", "pipelines", "SingleFrame.yaml")) | ||
) | ||
self.assertEqual( | ||
config.get_pipeline_file(dataclasses.replace(self.visit, survey="Camera Test")), | ||
os.path.normpath(os.path.join(getPackageDir("ap_pipe"), "pipe lines", "Isr.yaml")) | ||
) | ||
|
||
def test_nomatch(self): | ||
config = PipelinesConfig('(survey="TestSurvey")=/etc/pipelines/SingleFrame.yaml ' | ||
'(survey="CameraTest")=${AP_PIPE_DIR}/pipelines/Isr.yaml ' | ||
) | ||
with self.assertRaises(RuntimeError): | ||
config.get_pipeline_file(dataclasses.replace(self.visit, survey="Surprise")) | ||
|
||
def test_empty(self): | ||
with self.assertRaises(ValueError): | ||
PipelinesConfig('') | ||
with self.assertRaises(ValueError): | ||
PipelinesConfig(None) | ||
|
||
def test_commas(self): | ||
with self.assertRaises(ValueError): | ||
PipelinesConfig('(survey="TestSurvey")=/etc/pipelines/SingleFrame.yaml, ' | ||
'(survey="CameraTest")=${AP_PIPE_DIR}/pipelines/Isr.yaml ' | ||
) | ||
with self.assertRaises(ValueError): | ||
PipelinesConfig('(survey="TestSurvey")=/etc/pipelines/SingleFrame.yaml,' | ||
'(survey="CameraTest")=${AP_PIPE_DIR}/pipelines/Isr.yaml ' | ||
) | ||
|
||
def test_unlabeled(self): | ||
with self.assertRaises(ValueError): | ||
PipelinesConfig('(survey="TestSurvey")=/etc/pipelines/SingleFrame.yaml, ' | ||
'("CameraTest")=${AP_PIPE_DIR}/pipelines/Isr.yaml ' | ||
) | ||
|
||
def test_oddlabel(self): | ||
with self.assertRaises(ValueError): | ||
PipelinesConfig('(reason="TestSurvey")=/etc/pipelines/SingleFrame.yaml') | ||
|
||
def test_nospace(self): | ||
with self.assertRaises(ValueError): | ||
PipelinesConfig('(survey="TestSurvey")=/etc/pipelines/SingleFrame.yaml' | ||
'(survey="CameraTest")=${AP_PIPE_DIR}/pipelines/Isr.yaml' | ||
) | ||
with self.assertRaises(ValueError): | ||
PipelinesConfig('/etc/pipelines/SingleFrame.yaml' | ||
'(survey="CameraTest")=${AP_PIPE_DIR}/pipelines/Isr.yaml' | ||
) | ||
|
||
def test_noequal(self): | ||
with self.assertRaises(ValueError): | ||
PipelinesConfig('/etc/pipelines/SingleFrame.yaml') | ||
|
||
with self.assertRaises(ValueError): | ||
PipelinesConfig('/etc/pipelines/SingleFrame.yaml ' | ||
'(survey="CameraTest")=${AP_PIPE_DIR}/pipelines/Isr.yaml ' | ||
) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably just a matter of taste. Would
r'(?P<filename>[^=,]*[^=, ])(?:\s+|$)')
be slightly easier to read and still correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had that before, actually, but then got worried that that might match some unexpected (and possibly malicious) string. I figured it's best to err on the side of being too restrictive.
I agree that
^=,
would be easier to read.