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

CLI: cannot set a file path to data in workflow.json #639

Closed
josephjclark opened this issue Mar 21, 2024 · 5 comments · Fixed by #715
Closed

CLI: cannot set a file path to data in workflow.json #639

josephjclark opened this issue Mar 21, 2024 · 5 comments · Fixed by #715
Labels
good first issue Good for newcomers

Comments

@josephjclark
Copy link
Collaborator

josephjclark commented Mar 21, 2024

When running a workflow in the CLI, it is possible to set the expression (and I think credential) to a path.The CLI will then load this path when it loads the workflow.json.

This doesn't work for data at the moment, but it should. The data key sets the initial state for that step.

Example use-case:

{
    "workflow": {
        "steps": [
            {
                "id": "mapping",
                "adaptor": "common",
                "data": "./sampleData/data.json",
                "expression": "./jobs/parse-data.js"
            }
        ]
    }
}

Related: we may also want to support a state property on options, which would set the initial state if one isn't otherwise provided. This is probably a separate issue to be honest, but if I were working on it I would resolve both at once. I actually think the options thing is a more valuable use-case.

@josephjclark josephjclark added the good first issue Good for newcomers label May 15, 2024
@josephjclark
Copy link
Collaborator Author

We'll also have to update deploy to build the data inline into the workflow (like we do with expressions)

@SatyamMattoo
Copy link
Contributor

Hey @josephjclark,
I have worked on this issue, and it turns out that the data is within the state object. So, I have added a function that checks if the state is a file path. If it is, the function reads the file. Additionally, if the data (or any key inside state) is a file path, it will read that file and update the key with the file's content.

For example, if we have
workflow.json:

{
    "workflow": {
        "steps": [
            {
                "id": "1",
                "adaptor": "common",
                "state": "./state.json",
                "expression": "./expression.js"
            }
        ]
    }
}

state.json:

{
    "data":"./data.json"
}

data.json:

{
    "x": 1
}

It will compile the workflow as:

{
  "workflow": {
    "steps": [
      {
        "id": "1",
        "adaptor": "@openfn/language-common",
        "state": {
          "data": {
            "x": 1
          }
        },
        "expression": "import { fn } from \"@openfn/language-common\";\nexport * from \"@openfn/language-common\";\nexport default [fn((state) => state)];"
      }
    ],
    "name": "input"
  },
  "options": {}
}

I hope this is what we were aiming for. If there is anything I am missing or wrong about, please do correct me.

Best regards.

@josephjclark
Copy link
Collaborator Author

Hi @SatyamMattoo if you'd like to fix this issue please raise a PR and add your notes in there. It's OK for the pull request to be a draft or WIP or incomplete or whatever you want to call it. It's a lot easier for me to review and give feedback to you that way 🙏

It looks like your approach is correct but I'll have to dig my brain into it once the PR is opened.

Thanks!

@SatyamMattoo
Copy link
Contributor

Thank you for your response @josephjclark! I have raised the PR, you can review it at your convenience.

@josephjclark
Copy link
Collaborator Author

Thank you @SatyamMattoo !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants