Fix the status command#176
Merged
Merged
Conversation
Shell variables are essentially key value pairs that are associated with a `Batch`. It isn't a great idea to store lists within a db and as such each variable will be stored in its own table. The basic design of ShellVariable has been modelled of Job (w/o the running code)
To allow for greater flexibility in the arguments passing, the individual config files now dictate their arguments list. Note: The `config.args()` method is the safe version which handles the case when no arguments are specified. The `config.arguments` attribute returns the raw version from the config BUG FIX: It should have been `arg_n` not `arg_name`
There are going to be a couple of steps involved when creating a `batch`. Thus it is easier if they are abstracted away into a single function
It is only going to be used within the run method and as such should be located with it
The ShellVariable models need to be added to the batches in a similar way to Jobs. This does take some effort to combined the shell variables as key value pairs
This will prevent threading issue with SQLAlchemy
The shell variables are set as bash variables before the command is ran. This allows then to be substituted into the config command.
This is to prevent a injection attack through the shell variables
This allows the error to be caught by clicks top level error handler
The configs do not get passed for the `root` command BUT should be passed in all other cases
DavidMarchant
approved these changes
Nov 19, 2018
Contributor
DavidMarchant
left a comment
DavidMarchant
left a comment
There was a problem hiding this comment.
Yep LGTM! merge through soon as #175 is good to go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Based on #175
This is fairly simple fix to the status command. The
configsweren't being passed into the command, as such the rootstatuscommand would always run. Fixes #173