Skip to content
This repository has been archived by the owner on Nov 8, 2022. It is now read-only.

Fix #1129 - Add schedule validation check for task manifests in snapctl #1149

Merged
merged 2 commits into from
Aug 26, 2016
Merged

Fix #1129 - Add schedule validation check for task manifests in snapctl #1149

merged 2 commits into from
Aug 26, 2016

Conversation

geauxvirtual
Copy link
Contributor

@geauxvirtual geauxvirtual commented Aug 17, 2016

Fixes #1129

Summary of changes:

  • Validates a task from a task manifest includes a schedule and the schedule is not empty.
  • Moves the version check for a task manifest into a better function that includes other validations.

Testing done:

  • Verified passing a task manifest without a schedule returns an error.
  • Verified passing a task manifest with an empty schedule returns an error.

@intelsdi-x/snap-maintainers

if err := validateTask(t); err != nil {
return err
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should validateTask(t) be called after t.mergeCliOptions below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea is to validate that both a schedule and workflow were included in the task manifest file and throw an appropriate error before the code attempts to use certain aspects of the created task struct so that the user will receive a better error than runtime error: invalid memory address or nil pointer dereference.

@jcooklin
Copy link
Collaborator

LGTM - however this PR doesn't fix #1130 as it is worded. The issue exists if curl is used.

@geauxvirtual
Copy link
Contributor Author

Good catch. I can add to this PR some validation on the server side, or pull out the commits around #1130 and open a new PR with those commits plus the additional server side changes.

@geauxvirtual geauxvirtual changed the title Fix #1129 & #1130 - Add validation checks to snapctl for task and workflow manifests Fix #1129 - Add schedule validation check for task manifests in snapctl Aug 22, 2016
@jcooklin jcooklin merged commit ee89c87 into intelsdi-x:master Aug 26, 2016
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.

2 participants