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

Refine time-sync config logic #939

Closed
2 of 3 tasks
jmcook1186 opened this issue Aug 6, 2024 · 1 comment · Fixed by #945
Closed
2 of 3 tasks

Refine time-sync config logic #939

jmcook1186 opened this issue Aug 6, 2024 · 1 comment · Fixed by #945
Assignees

Comments

@jmcook1186
Copy link
Contributor

jmcook1186 commented Aug 6, 2024

What
Update current logic so that time-sync applies aggregationMethod == None when it encounters an unknown parametr, rather than having to have parameters explicitly defined in the manifest and passed as arguments.

Why
The current behaviour requires users to pass unknown parameters by name to the timeSync plugin in order to avoid time-sync erroring out. This means affected users have to go through a mini-debugging process to get time sync to execute. Instead, we should just fallback to a default behaviour where unspecified values are just copied into each new timestep and a simple warning is emitted.

Note that this is to fix an edge case scenario where values are provided in defaults but not actually used in pipelines, so metadata is not available for them. Most scenarios are handled via the happy path which is that parameter metadata is available in plugin source code or the manifest's initialize block.

Context
n/a

Prerequisites/resources
n/a

SoW (scope of work)

  • update time-sync logic to fallback to none where metadata cannot be found
  • documentation updated
  • test cases added

Acceptance criteria

Given (Setup): the time-sync logic falls back to aggregation-method == None where no metadata can be found for a parameter.

When (Action): I run the following manifest:

name: time-converter demo
description: successful path
tags: null
initialize:
  plugins:
    time-sync:
      path: builtin
      method: TimeSync
      global-config:
        start-time: '2023-08-06T00:00:00.000Z'
        end-time: '2023-08-06T00:01:00.000Z'
        interval: 10
        allow-padding: true
tree:
  children:
    child:
      pipeline:
        - time-sync
      config: null
      defaults:
        param-without-metadata: 'hello'
      inputs:
        - timestamp: 2023-08-06T00:00:00.000Z
          duration: 1
        - timestamp: 2023-08-06T00:01:00.000Z
          duration: 1

Then (Assertion): Defines the expected outcome or behavior of the system after the action in the "When" step is performed.

name: time-converter demo
description: successful path
tags: null
initialize:
  plugins:
    time-sync:
      path: builtin
      method: TimeSync
      global-config:
        start-time: '2023-08-06T00:00:00.000Z'
        end-time: '2023-08-06T00:01:00.000Z'
        interval: 10
        allow-padding: true
execution:
  command: >-
    /home/joe/.npm/_npx/1bf7c3c15bf47d04/node_modules/.bin/ts-node
    /home/joe/Code/if/src/if-run/index.ts -m
    manifests/examples/builtins/time-converter/success.yaml
  environment:
    if-version: 0.5.0
    os: linux
    os-version: 5.15.0-117-generic
    node-version: 21.4.0
    date-time: 2024-08-06T13:12:08.867Z (UTC)
    dependencies:
    ...
  status: success
tree:
  children:
    child:
      pipeline:
        - time-sync
      config: null
      defaults:
        param-without-metadata: 'hello'
      inputs:
        - timestamp: 2023-08-06T00:00:00.000Z
          duration: 1
        - timestamp: 2023-08-06T00:01:00.000Z
          duration: 1
      outputs:
        - timestamp: '2023-08-06T00:00:00.000Z'
          duration: 10
          param-without-metadata: hello
        - timestamp: '2023-08-06T00:00:10.000Z'
          duration: 10
          param-without-metadata: hello
        - timestamp: '2023-08-06T00:00:20.000Z'
          duration: 10
          param-without-metadata: hello
        - timestamp: '2023-08-06T00:00:30.000Z'
          duration: 10
          param-without-metadata: hello
        - timestamp: '2023-08-06T00:00:40.000Z'
          duration: 10
          param-without-metadata: hello
        - timestamp: '2023-08-06T00:00:50.000Z'
          duration: 10
          param-without-metadata: hello
        - timestamp: '2023-08-06T00:01:00.000Z'
          duration: 1
          param-without-metadata: hello
@jmcook1186 jmcook1186 added this to IF Aug 6, 2024
@jmcook1186 jmcook1186 moved this to Ready in IF Aug 6, 2024
@zanete zanete added this to the Inputs and Outputs milestone Aug 6, 2024
@zanete
Copy link

zanete commented Aug 6, 2024

a note that this should get done before the release fyi @narekhovhannisyan

@narekhovhannisyan narekhovhannisyan moved this from Ready to In Progress in IF Aug 8, 2024
@narekhovhannisyan narekhovhannisyan linked a pull request Aug 8, 2024 that will close this issue
9 tasks
@github-project-automation github-project-automation bot moved this from In Progress to Done in IF Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants