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

Implement Variable API #242

Merged
merged 21 commits into from
Feb 16, 2022
Merged

Implement Variable API #242

merged 21 commits into from
Feb 16, 2022

Conversation

philippjfr
Copy link
Member

@philippjfr philippjfr commented Feb 9, 2022

It is frequently required to declare variables that will be reused throughout the dashboard specification. Currently we have support for declaring special variables using yaml templating syntax, e.g. to resolve environment variables we can use the jinja2 syntax {{ env('some_env_var') }} and this is resolved when the dashboard is loaded. However frequently we have the need to dynamically resolve variables, tie variables to widgets, or specifically in the context of the Lumen Builder to wait to resolve the variables until the final dashboard. To empower users to do these things in a very explicit way we add a Variable hierarchy to the existing hierarchy of components (i.e. in addition to Filter, Source, Transform and View).

Just like all other components users will be able to declare their own Variable types, e.g. to load some secret from a custom variable storage system. Some additional design considerations:

  1. The Variable definitions are the first thing that are resolved ensuring that all other components can make use of the variables. One Variable may even reference another Variable in its definition as long as they are declared in the appropriate order.

  2. To use a variable anywhere in the yaml specification you use the $ syntax already used by sources, e.g. to reference a Variable named username you would specify $variable.username (alternative suggestions welcome)

  3. A Variable can be dynamic just like any other component, therefore it may implement a panel attribute or property that returns a Panel component which will be rendered in the sidebar.

  4. A Variable can declare whether it is to be inlined/resolved or not. This is a declaration to the Lumen Builder whether it should resolve the variable and replace it with a ConstantVariable or whether it should simply be inherited as is.

@jbednar
Copy link
Member

jbednar commented Feb 9, 2022

This seems like a solid solution to a lot of the problems we're wrestling with. Can you add an example .yml file showing how to use this, if only as a comment here? Personally I find $ to indicate variable substitution more clearly than @, which has gotten a connotation of referring to a username, but there may be YAML precedents here that lend weight to @.

@codecov-commenter
Copy link

codecov-commenter commented Feb 10, 2022

Codecov Report

Merging #242 (61574b3) into master (d89cb11) will increase coverage by 1.74%.
The diff coverage is 88.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #242      +/-   ##
==========================================
+ Coverage   61.84%   63.59%   +1.74%     
==========================================
  Files          52       56       +4     
  Lines        4739     5021     +282     
==========================================
+ Hits         2931     3193     +262     
- Misses       1808     1828      +20     
Impacted Files Coverage Δ
lumen/util.py 57.77% <ø> (ø)
lumen/state.py 69.42% <71.87%> (+3.12%) ⬆️
lumen/variables/base.py 78.83% <78.83%> (ø)
lumen/target.py 74.86% <80.00%> (+1.47%) ⬆️
lumen/dashboard.py 76.67% <83.33%> (+0.10%) ⬆️
lumen/transforms/base.py 80.60% <90.90%> (-0.38%) ⬇️
lumen/sources/base.py 58.14% <93.33%> (+0.20%) ⬆️
lumen/base.py 96.87% <96.87%> (ø)
lumen/filters/base.py 64.83% <100.00%> (-1.66%) ⬇️
lumen/tests/conftest.py 100.00% <100.00%> (ø)
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d89cb11...61574b3. Read the comment docs.

@philippjfr philippjfr merged commit 4e46494 into master Feb 16, 2022
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants