-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
IOError traceback using malformed commandline #6538
Comments
On 06-08-2013 16:11, Mrten wrote:
This is not a problem with salt. It's a shell problem. Please try: |salt '(xxx|yyy)*' test.ping| I think those quotes should do the job. Regards,Pedro Algarvio Pedro Algarvio |
I know it is an invalid command, however, salt shouldn't trackback on it IMHO... |
@Mrten You and your invalid commands....... ;) I guess we'll fix your traceback. =D |
Yes, I agree, this is trivial, no, as such it is not important (except that one of you wrote somewhere that all tracebacks should be reported, but hey). But it's part of a more global trend within salt that I'm noticing, so, if you'll allow me to get on my soapbox for a minute: I've noticed during six or seven months that I've been using salt now that it has a tendency to not validate (or incompletely validate) the user input. When you use salt "as intended by the developer" everything works out OK, but as soon as you step outside the box that has been tested you very soon enter "here be dragons"-land and unexpected things happen. This bug report is an example of it, I think I've reported some more, others have, too. As a programmer where I work validating user input is something I've been hammering on endlessly to my team: You just cannot trust what the user gives you, nor should you accept it blindly. I've been hammering on it so long that each traceback that I see when I make a mistake irks me so much I file bugreports ;) Salt should be secure by default. If the commandline isn't 100% correct, die with a warning. Forget something in a state, die with a warning. Unicode where only ASCII is valid? Gone. I realize this may not align completely with your current goals (you guys are going awfully fast with salt), but at least you know now where I'm coming from. I'll get off my soapbox now, thank you for listening :) |
@Mrten I agree with your soapbox! We would love to carefully validate user input, but like you say, Salt moves very fast, and right now we just don't have the bandwidth. Luckily, we have users like you that find the edge cases for us! =D |
So I'm looking this over again, and I'm really unsure of how to fix it. I mean, I could wrap that line in salt/utils/parsers.py with a try:except:, and yes, that would get rid of the traceback, but is that what we want? The traceback itself didn't kill the minion -- rather it just reported the error in the log and went on its merry way. What I'm worried about is that by putting a try:except: around that line and claiming to know what caused the error, I'm going to mis-diagnose errors in the future. (We had a similar problem with TypeErrors in states -- they would come back as this weird argspec message, because we were catching exceptions too broadly and assuming we knew what caused them) I'm not saying this isn't a problem, I'm just trying to find an elegant way to resolve this without causing problems down the road. |
how about an |
@cachedout this is a dup of another issue assigned to you, exact same thing, we should be able to catch it with an os.error |
… to us that even Python's optparse can't deal with. Refs saltstack#6538.
Make an effort to return a warning message if we get arguments passed to...
… to us that even Python's optparse can't deal with. Refs #6538.
salt \(xxx|yyy\)* test.ping
The text was updated successfully, but these errors were encountered: