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

Status task pipelines save with improper json #1493

Open
remkonoteboom opened this issue Jun 22, 2020 · 2 comments
Open

Status task pipelines save with improper json #1493

remkonoteboom opened this issue Jun 22, 2020 · 2 comments
Labels

Comments

@remkonoteboom
Copy link
Contributor

Describe the bug
The json saved is not correct when saving a task pipeline. It looks something like this:

{
  "default": {
    "color": "#fa2929",
    "mapping": "Complete",
    "node_type": "manual"
  },
  "mapping": "Complete",
  "node_type": "status",
  "version": 2
}

Note that this is harmless, but should be fixed at some point.

To Reproduce
Steps to reproduce the behavior:

  1. Create a task pipeline
  2. Save
  3. Look at the workflow column on the process table for this pipeline

Expected behavior
The json should look something like this:

{
  "default": {
    "color": "#fa2929",
    "mapping": "Complete",
  },
  "node_type": "status",
  "version": 2
}
@diegocortassa
Copy link
Contributor

The mapping, direction and status keys are saved both in "default" and at the root level by code I committed in da69a44 because they are used by "src/pyasm/command/workflow.py"

in lines 3151-3152

            workflow_data = process_sobj.get_json_value("workflow", {})
            mapping = workflow_data.get("mapping")

line 3152 should be:
mapping = workflow_data.get("default").get("mapping")

same problem in lines 3158,3159 and 3160

I was unsure if changing workflow.py was the correct way to solve the problem and if "default" is the correct key because manual node for example use "properties" instead of "default"

Is "default" the right key for the status node datas?
If this is the case is it ok to send a PR changing src/pyasm/command/workflow.py to look for data under the "default" key?

@remkonoteboom
Copy link
Contributor Author

remkonoteboom commented Jul 8, 2020

Sorry for taking so long to answer this. I missed this comment.

We moved to having named dictionaries because the amount of data being stored by the various parts were getting to large (this was especially true on some of our internal tools). So we moved all the old settings for the manual node to "properties". This is an arbitrary key and as long as all the parts look at the same place, it's ok.

We apparently didn't fix the status nodes for the task pipeline to update with this. Whether we use "properties" or "default" doesn't matter. What I would prefer is that no data except "version" and "node_type" lies in the root of the "workflow" dictionary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants