Skip to content
This repository has been archived by the owner on Sep 16, 2020. It is now read-only.

Improve error handling for users using the schema command #750

Merged
merged 1 commit into from Mar 12, 2020

Conversation

AlanCoding
Copy link
Member

It looks like the server stopped verifying that unified_job_template has a non-null value because of the additional of approval nodes.

This has a consequence for function of the schema command, since it was never documented how to use workflows-in-workflows. It would silently finish with null nodes, which is a sideways state.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 64.064% when pulling fb1772b on AlanCoding:schema_error_handling into b87d8ea on ansible:master.

ujt_ambiguity_msg = (
'You should provide exactly one of the attributes'
' job_template, project, workflow, inventory_source, or unified_job_template.'
)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is way better wording!

@@ -45,16 +45,19 @@ def __init__(self, data, wfjt, include_id=False):
else:
node_attrs[fd] = data[fd]
node_attrs['workflow_job_template'] = wfjt
ujt_ambiguity_msg = (
'You should provide exactly one of the attributes'
' job_template, project, workflow, inventory_source, or unified_job_template.'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 for this clear message. Although I am not familiar with unified_job_template

@@ -127,6 +127,16 @@ network with a tower-cli command after the constituent resources like
the job templates and projects were created by preceding tower-cli
commands.

Workflows Inside Workflows
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good. How about adding workflow above as well in the example https://github.com/ansible/tower-cli/pull/750/files#diff-e888d13496218a3618bbee8be0d2a7d1R114-R124 ? I see example already shows jt, project, inv_source, so we can add workflow there and make it a complete example. Just a suggestion.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope you don't mind that I don't do that out of sheer laziness. I know that I've tested the content here is functional. The implication is that you can combine them all together, but we just don't want poke this library any more than we have to.

@AlanCoding AlanCoding merged commit 29b27f4 into ansible:master Mar 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants