Check if a workflow payload is valid during sync #59
Conversation
|
I think it would be cool in the UI if you could see the payload_valid as a ✅ or ❌ in the table, then perhaps have the error on mouse over. Or, at a minimum show the value on the summary page. |
|
I really want to write some specs for this, the way the fake repo is set up it makes it difficult to setup an invalid file for a single test. Going to play with that a little bit |
| found.update!(:name => filename, :manager_id => manager_id, :payload => payload, :payload_type => "json") | ||
| payload_valid, payload_error = | ||
| begin | ||
| Floe::Workflow.new(payload) |
There was a problem hiding this comment.
If the payload is very broken, like bad JSON broken, will it still raise a Floe::InvalidWorkflowError ?
There was a problem hiding this comment.
|
I was just thinking - with the new column, we defaulted to valid (because I suggested it), but do we want a tri-boolean column? 😉...in all seriousness though, I could see valid, invalid and unknown, and the ansible ones could be unknown? I started thinking about my comment visualizing them in a table, and then I got to thinking how we want to collapse these views together and it might be weird to see ansible as "valid" even though we didn't verify them? Thoughts? |
|
Hm would It be weird that |
|
Yeah I was thinking a gray question mark. Alternatively we could always add some basic ansible validation. (YAML parses, has a certain structure, etc) |
agrare
force-pushed
the
check_workflows_payload_validity
branch
from
a975aba to
fc8709e
Compare
| def each_file | ||
| git_repository.update_repo | ||
| git_repository.with_worktree do |worktree| | ||
| worktree.ref = scm_branch | ||
| worktree.blob_list.each do |filename| | ||
| next if filename.start_with?(".") || !filename.end_with?(".asl") | ||
|
|
||
| payload = worktree.read_file(filename) | ||
| yield filename, payload | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
Think this would be better in core's EmbeddedAutomationManager repository class but I wanted to test it out self-contained here.
|
This pull request has been automatically marked as stale because it has not been updated for at least 3 months. If these changes are still valid, please remove the |
agrare
force-pushed
the
check_workflows_payload_validity
branch
from
ce29cbd to
4c83d36
Compare
|
Can we merge this even with valid column being true/false? |
When syncing a repository and creating workflows check if the payload is valid and if it isn't set a (hopefully) helpful error message.
agrare
force-pushed
the
check_workflows_payload_validity
branch
from
4c83d36 to
6275276
Compare
|
Checked commit agrare@6275276 with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint |
|
This pull request is not mergeable. Please rebase and repush. |
|
This pull request has been automatically marked as stale because it has not been updated for at least 3 months. If these changes are still valid, please remove the |
|
This pull request has been automatically marked as stale because it has not been updated for at least 3 months. If these changes are still valid, please remove the |
When syncing a repository and creating workflows check if the payload is valid and if it isn't set a (hopefully) helpful error message.
Depends on:
TODO: