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

Update response of flow run list #4444

Closed
milanholemans opened this issue Jan 30, 2023 · 2 comments
Closed

Update response of flow run list #4444

milanholemans opened this issue Jan 30, 2023 · 2 comments

Comments

@milanholemans
Copy link
Contributor

milanholemans commented Jan 30, 2023

Was checking the command flow run list and noticed 2 things that are not quite optimal.

  1. Noticed that the command flow run list is modifying the result. We copy the startTime and status to another place in the result. This for the simple reason that these are defaultProperties and thus won't work when they are in a nested object. According to me it's quite pointless to have 2 the same properties in the result.
    Therefore I suggest that we perform an output check, and if we should modify the output (text, csv), that we return a custom response with all defaultProperties in it. If this is not the case, just return the original result

  2. We only log the result if we get at least 1 flow run returned. If there are no runs returned, we simply log an error and that's it.
    According to me, if there were no flow runs found, we should just log an empty array and that's it, it's quite hard to use this command in a script if it can return either an array or an error.

The code right now:

if (this.items.length > 0) {
this.items.forEach(i => {
i.startTime = i.properties.startTime;
i.status = i.properties.status;
});
logger.log(this.items);
}
else {
if (this.verbose) {
logger.logToStderr('No runs found');
}
}

Update:

Same goes for following commands:

  • flow environment list
  • flow environment get
@martinlingstuyl
Copy link
Contributor

Opening this up as we are releasing v7 after the summer holidays. Warmly recommended for your contribution. 🎉
Target the v7 branch when raising a PR.

@MathijsVerbeeck
Copy link
Contributor

🙋‍♂️

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