alficles commented on issue #2072: Protect ORT from errors caused by missing data in TO. URL: https://github.com/apache/incubator-trafficcontrol/pull/2072#issuecomment-377816753 Right, I thought about that. Right now, the behaviour is to barf out perl errors and forge ahead ON ERROR RESUME NEXT–style. At the moment, the effect is to just not process that config file, which is likely to cause serious errors if that file is required for anything important. I worry, however, that the lack of error handling could cause more serious errors in the future. One could easily imagine a scenario where a directory is being configured instead of a file and we `chown ats:ats $config_dir/*`. If `$config_dir` is empty, that could cause all manner of trouble executing as root. In any case, barfing errors and continuing isn't great. I considered having ORT bail entirely. The problem here is that it's already accomplished some of its work before it discovers the error. And all the rest of the work can still be safely accomplished. Sure, you might wind up with ATS refusing to start because it's missing `foobaz.lua` which is required for a specific plugin. But then at least the file reported as missing is the one that isn't properly configured. If we just bail when we see the ill-configured file, we run the risk fo ATS refusing to start because it doesn't have `goobaz.lua`, which is properly configured but would have been handled right after `foobaz.lua`. In practice, I've also observed that this error isn't always as fatal as you might guess. A config file will be added for a new plugin on a new Delivery Service, but the DS won't be assigned to anything important until it's been tested. But the config file will be part of the profile for lots of things. During this (sometimes lengthy) period, there can be a misconfigured config file in the profile that isn't actually damaging production traffic. I guess the ideal scenario is that ORT parses through all the files it needs and ensures that it has all the data before it starts any work at all, and refuses to do anything unless everything is copasetic. That's a great feature and someone should write it. :) This, however, makes the code just a little bit safer and the error message considerably easier to understand.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
