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

env option parsing changes #1711

Open
shinybrar opened this issue Feb 18, 2025 · 1 comment
Open

env option parsing changes #1711

shinybrar opened this issue Feb 18, 2025 · 1 comment
Assignees

Comments

@shinybrar
Copy link
Contributor

In previous firefly versions which utilize a bash script entry-point, the following Kubernetes manifest was supported for setting PROPS_FIREFLY_OPTIONS environment variable using bash-specific quoting $'…'

- name: "PROPS_FIREFLY_OPTIONS"
  value: >-
    $'{"tap": <<<<<------- NO LONGER WORKS
        {"additional":
          {"services":[
            {
              "label": "CADC YouCAT",
              "value": "https://ws-uv.canfar.net/youcat",
              "centerWP": "62;-37;EQ_J2000",
              "fovDeg": 10
            }]
        }
      }
    }'

However, in the updated version of firefly, with a python entry-point parsing is handled by the python module shlex.quote, only plain JSON format (without bash-specific quoting) is parsed correctly and does
not interpret the bash-specific $'…' syntax, leading to extra quoting that results in malformed JSON.

- name: "PROPS_FIREFLY_OPTIONS"
  value: >-
    {"tap":  <<<<<------- WORKS AS EXPECTED
        {"additional":
          {"services":[
            {
              "label": "CADC YouCAT",
              "value": "https://ws-uv.canfar.net/youcat",
              "centerWP": "62;-37;EQ_J2000",
              "fovDeg": 10
            }]
        }
      }
    }

Could we update the documentation to inform users of this change? Providing guidance on using the plain JSON format for PROPS_FIREFLY_OPTIONS will help users transition smoothly and avoid any potential issues when upgrading to the new version, e.g. lsst/suit implementation will break with the new release.

@robyww
Copy link
Contributor

robyww commented Feb 18, 2025

We can, but this version is not released yet and Rubin is not even using it. I have not even tried this new stuff in rubin phalanx statefulset.yaml

@robyww robyww self-assigned this Feb 18, 2025
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

No branches or pull requests

2 participants