)]}'
{"doc/source/main_features.rst":[{"author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"change_message_id":"14e874574c8202cb7a89cc07092d1182c27f01f7","unresolved":false,"context_lines":[{"line_number":295,"context_line":" hours Mistral will clean up and look for expired executions."},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"std.command action definition"},{"line_number":298,"context_line":"-----------------------------"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"``std.command`` action definition has been supported by Mistral out of the box"},{"line_number":301,"context_line":"since Ocata, just as other standard actions that pre-defined in Mistral. This"}],"source_content_type":"text/x-rst","patch_set":6,"id":"5a8bc5a2_6ae7a2ac","line":298,"updated":"2016-09-28 11:07:51.000000000","message":"It would be good to say that this is disabled by default, unless a scripts folder is added to the config.","commit_id":"763d9cf97de80db25c11207e29b90cf7854a2fea"},{"author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"change_message_id":"14e874574c8202cb7a89cc07092d1182c27f01f7","unresolved":false,"context_lines":[{"line_number":303,"context_line":"Mistral. This action definition is supposed to be running scripts on the host"},{"line_number":304,"context_line":"where ``mistral-executor`` service is running on. Currently, only python and"},{"line_number":305,"context_line":"bash scripts are supported. Since executing user defined scripts inside cloud"},{"line_number":306,"context_line":"is indeed a risky thing and may potentially lead to security issues, we"},{"line_number":307,"context_line":"recommend using this action in private cloud, and cloud operator should config"},{"line_number":308,"context_line":"a folder that contains all the scripts, and most importantly, operators should"},{"line_number":309,"context_line":"know exactly what those scripts are doing when they are being executed. The"}],"source_content_type":"text/x-rst","patch_set":6,"id":"5a8bc5a2_aada8ae7","line":306,"updated":"2016-09-28 11:07:51.000000000","message":"There is no way that restricting to bash or Python will improve security. Both of these are capable of calling any other executable. I still think this restriction is a mistake and doesn\u0027t make sense.","commit_id":"763d9cf97de80db25c11207e29b90cf7854a2fea"},{"author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"change_message_id":"14e874574c8202cb7a89cc07092d1182c27f01f7","unresolved":false,"context_lines":[{"line_number":308,"context_line":"a folder that contains all the scripts, and most importantly, operators should"},{"line_number":309,"context_line":"know exactly what those scripts are doing when they are being executed. The"},{"line_number":310,"context_line":"scripts folder can contain sub-folders based on the scripts category, but"},{"line_number":311,"context_line":"please make the scripts file name unique to avoid unexpected result. At the"},{"line_number":312,"context_line":"meanwhile, the resource that the script can use is also limited and can be"},{"line_number":313,"context_line":"configure."},{"line_number":314,"context_line":""}],"source_content_type":"text/x-rst","patch_set":6,"id":"5a8bc5a2_aaafea29","line":311,"updated":"2016-09-28 11:07:51.000000000","message":"I aslo think this is confusing and a mistake. Given that we need people to read this to make sense of it it will catch lots of users out. Nobody reads the documentation first.","commit_id":"763d9cf97de80db25c11207e29b90cf7854a2fea"},{"author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"change_message_id":"14e874574c8202cb7a89cc07092d1182c27f01f7","unresolved":false,"context_lines":[{"line_number":322,"context_line":"    script_cpu_time \u003d 300"},{"line_number":323,"context_line":"    script_address_space \u003d 1"},{"line_number":324,"context_line":"    script_number_files \u003d 5"},{"line_number":325,"context_line":"    script_number_processes \u003d 5"}],"source_content_type":"text/x-rst","patch_set":6,"id":"5a8bc5a2_aa88cac7","line":325,"updated":"2016-09-28 11:07:51.000000000","message":"I had expected these to be arguments for the action - I can see a use-case where I would want different time limits for different scripts.","commit_id":"763d9cf97de80db25c11207e29b90cf7854a2fea"}],"mistral/actions/std_actions.py":[{"author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"change_message_id":"0f5c32cc4409e5b23cfc937466c02aa97a9b145c","unresolved":false,"context_lines":[{"line_number":489,"context_line":"    - Integrate with Swift so that this action could be used in public cloud"},{"line_number":490,"context_line":"      under end user\u0027s context."},{"line_number":491,"context_line":"    \"\"\""},{"line_number":492,"context_line":"    SUPPORTED_COMMANDS \u003d {\u0027bash\u0027: \u0027/bin/bash\u0027, \u0027python\u0027: \u0027/usr/bin/python\u0027}"},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"    def __init__(self, command, path, params\u003d[], exit_code\u003d0):"},{"line_number":495,"context_line":"        if command not in self.SUPPORTED_COMMANDS:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_7c00e001","line":492,"updated":"2016-09-21 15:41:39.000000000","message":"It might be better to require that the file is callable, and have it pick the command based on it\u0027s shebang. Then we can support whatever callable is on the host machine under any path the sysadmin chooses.","commit_id":"732f99d97f830b81e4b5f6cdc2ed68393751db2f"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"74527d98bb8336b160c2cb221ca7b1e536b89b0a","unresolved":false,"context_lines":[{"line_number":489,"context_line":"    - Integrate with Swift so that this action could be used in public cloud"},{"line_number":490,"context_line":"      under end user\u0027s context."},{"line_number":491,"context_line":"    \"\"\""},{"line_number":492,"context_line":"    SUPPORTED_COMMANDS \u003d {\u0027bash\u0027: \u0027/bin/bash\u0027, \u0027python\u0027: \u0027/usr/bin/python\u0027}"},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"    def __init__(self, command, path, params\u003d[], exit_code\u003d0):"},{"line_number":495,"context_line":"        if command not in self.SUPPORTED_COMMANDS:"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a8bc5a2_be30f5e9","line":492,"in_reply_to":"5a8bc5a2_03c7f712","updated":"2016-09-23 04:22:33.000000000","message":"It\u0027s totally different that we require user to write well formatted yaml workflow and we require them how to write their own scripts. User must do things according to API, a well formatted yaml file is a \u0027protocol\u0027 based on what that they communicate with Mistral.\n\nThe scripts are not part of Mistral, users may write whatever they want, it\u0027s not part of Mistral \u0027protocol\u0027. The truth is, It\u0027s not difficult to ask users to do that, it\u0027s impossible to ask users to write scripts according to the way a 3rd party service (like Mistral) defined.\n\nWhat\u0027s more, what if we support Java command in future? user may upload compiled java class to swift and run, how can we figure out which command to use?","commit_id":"732f99d97f830b81e4b5f6cdc2ed68393751db2f"},{"author":{"_account_id":8532,"name":"Ryan Brady","email":"rbrady@redhat.com","username":"rbrady"},"change_message_id":"147ae60b060265a603e724887cff2feede12e66f","unresolved":false,"context_lines":[{"line_number":489,"context_line":"    - Integrate with Swift so that this action could be used in public cloud"},{"line_number":490,"context_line":"      under end user\u0027s context."},{"line_number":491,"context_line":"    \"\"\""},{"line_number":492,"context_line":"    SUPPORTED_COMMANDS \u003d {\u0027bash\u0027: \u0027/bin/bash\u0027, \u0027python\u0027: \u0027/usr/bin/python\u0027}"},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"    def __init__(self, command, path, params\u003d[], exit_code\u003d0):"},{"line_number":495,"context_line":"        if command not in self.SUPPORTED_COMMANDS:"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a8bc5a2_03c7f712","line":492,"in_reply_to":"5a8bc5a2_0aff0c02","updated":"2016-09-22 14:19:48.000000000","message":"\u003eWe can not make assumption that users could write scripts as we expected. Let the users do what they want.\n\nI think we can.  We expect users to write well formatted yaml workflows, right?  The users would need to add command\u003d\"python\" to the action: in the task in the yaml.\n\nIs it really more difficult to ask a user to add a single line of code to direct the OS which interpreter to use?\n\n#!/usr/bin/env python\nprint(\u0027Hello, World\u0027)\n\nThe benefits seem clear to me here:\n- reduced configuration\n- reduced code\n- extensibibility (language support can easily be added)","commit_id":"732f99d97f830b81e4b5f6cdc2ed68393751db2f"},{"author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"change_message_id":"ee2155571c498115af04f2b015a97b041a5ccaa8","unresolved":false,"context_lines":[{"line_number":489,"context_line":"    - Integrate with Swift so that this action could be used in public cloud"},{"line_number":490,"context_line":"      under end user\u0027s context."},{"line_number":491,"context_line":"    \"\"\""},{"line_number":492,"context_line":"    SUPPORTED_COMMANDS \u003d {\u0027bash\u0027: \u0027/bin/bash\u0027, \u0027python\u0027: \u0027/usr/bin/python\u0027}"},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"    def __init__(self, command, path, params\u003d[], exit_code\u003d0):"},{"line_number":495,"context_line":"        if command not in self.SUPPORTED_COMMANDS:"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a8bc5a2_edee7aeb","line":492,"in_reply_to":"5a8bc5a2_0aff0c02","updated":"2016-09-22 13:48:56.000000000","message":"They should put this in the file, which allows them to pick the language and version.\n\n    #!/usr/bin/env python3\n    print(\"Hello, World\")","commit_id":"732f99d97f830b81e4b5f6cdc2ed68393751db2f"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"c0aedadcdd54cdbdb772ed69e53aa19b587a6a85","unresolved":false,"context_lines":[{"line_number":489,"context_line":"    - Integrate with Swift so that this action could be used in public cloud"},{"line_number":490,"context_line":"      under end user\u0027s context."},{"line_number":491,"context_line":"    \"\"\""},{"line_number":492,"context_line":"    SUPPORTED_COMMANDS \u003d {\u0027bash\u0027: \u0027/bin/bash\u0027, \u0027python\u0027: \u0027/usr/bin/python\u0027}"},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"    def __init__(self, command, path, params\u003d[], exit_code\u003d0):"},{"line_number":495,"context_line":"        if command not in self.SUPPORTED_COMMANDS:"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a8bc5a2_0aff0c02","line":492,"in_reply_to":"5a8bc5a2_eb03cf74","updated":"2016-09-22 13:28:24.000000000","message":"Think about a simple example, what if user upload a file to Swift, with content:\n\n    print(\u0027Hello, World\u0027)\n\nHow do you know you should use \u0027python\u0027 or \u0027bash\u0027 in this case? We can not make assumption that users could write  scripts as we expected. Let the users do what they want.","commit_id":"732f99d97f830b81e4b5f6cdc2ed68393751db2f"},{"author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"change_message_id":"a751277dc295bc0188499eb1ab8b819c347132b0","unresolved":false,"context_lines":[{"line_number":489,"context_line":"    - Integrate with Swift so that this action could be used in public cloud"},{"line_number":490,"context_line":"      under end user\u0027s context."},{"line_number":491,"context_line":"    \"\"\""},{"line_number":492,"context_line":"    SUPPORTED_COMMANDS \u003d {\u0027bash\u0027: \u0027/bin/bash\u0027, \u0027python\u0027: \u0027/usr/bin/python\u0027}"},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"    def __init__(self, command, path, params\u003d[], exit_code\u003d0):"},{"line_number":495,"context_line":"        if command not in self.SUPPORTED_COMMANDS:"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a8bc5a2_eb03cf74","line":492,"in_reply_to":"7a8ec9b2_0ff4e358","updated":"2016-09-22 10:57:33.000000000","message":"I don\u0027t think so. With a shebang you should be able to do it even in the case of swift.\n\nswift download $container $file\nchmod +x $file\n./$file","commit_id":"732f99d97f830b81e4b5f6cdc2ed68393751db2f"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"01dd35bbdf620b3bb706ef6c1be2653e0f25db2e","unresolved":false,"context_lines":[{"line_number":489,"context_line":"    - Integrate with Swift so that this action could be used in public cloud"},{"line_number":490,"context_line":"      under end user\u0027s context."},{"line_number":491,"context_line":"    \"\"\""},{"line_number":492,"context_line":"    SUPPORTED_COMMANDS \u003d {\u0027bash\u0027: \u0027/bin/bash\u0027, \u0027python\u0027: \u0027/usr/bin/python\u0027}"},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"    def __init__(self, command, path, params\u003d[], exit_code\u003d0):"},{"line_number":495,"context_line":"        if command not in self.SUPPORTED_COMMANDS:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_0ff4e358","line":492,"in_reply_to":"7a8ec9b2_7c00e001","updated":"2016-09-22 02:08:55.000000000","message":"I was thinking about how to provide such action to public cloud users, e.g user may have an object stored in Swift, and that object content is actually a python script, we need let user tell Mistral what kind of script the user wants to run.","commit_id":"732f99d97f830b81e4b5f6cdc2ed68393751db2f"},{"author":{"_account_id":8532,"name":"Ryan Brady","email":"rbrady@redhat.com","username":"rbrady"},"change_message_id":"5b4ef333cf7303d3e8d9b0992456b21d4fa8b5b3","unresolved":false,"context_lines":[{"line_number":494,"context_line":"    def __init__(self, command, path, params\u003d[], exit_code\u003d0):"},{"line_number":495,"context_line":"        if command not in self.SUPPORTED_COMMANDS:"},{"line_number":496,"context_line":"            raise exc.ActionException("},{"line_number":497,"context_line":"                \u0027%s is not supported, current supported commands: %s\u0027 %"},{"line_number":498,"context_line":"                (command, self.SUPPORTED_COMMANDS)"},{"line_number":499,"context_line":"            )"},{"line_number":500,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_7cde2012","line":497,"range":{"start_line":497,"start_character":38,"end_line":497,"end_character":64},"updated":"2016-09-21 15:59:30.000000000","message":"if I am running mistral without keystone auth, I\u0027m not sure I want to notify users which commands are supported.  It might mean fewer log entires to find suspicious behavior.","commit_id":"732f99d97f830b81e4b5f6cdc2ed68393751db2f"},{"author":{"_account_id":8532,"name":"Ryan Brady","email":"rbrady@redhat.com","username":"rbrady"},"change_message_id":"69c9106a0e438dc199b2d3a60e44d3d9b94e627c","unresolved":false,"context_lines":[{"line_number":494,"context_line":"    def __init__(self, command, path, params\u003d[], exit_code\u003d0):"},{"line_number":495,"context_line":"        if command not in self.SUPPORTED_COMMANDS:"},{"line_number":496,"context_line":"            raise exc.ActionException("},{"line_number":497,"context_line":"                \u0027%s is not supported, current supported commands: %s\u0027 %"},{"line_number":498,"context_line":"                (command, self.SUPPORTED_COMMANDS)"},{"line_number":499,"context_line":"            )"},{"line_number":500,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5a8bc5a2_2af3e874","line":497,"in_reply_to":"5a8bc5a2_4b8843e8","updated":"2016-09-22 13:37:07.000000000","message":"\u003e I don\u0027t see a problem with revealing the commands. If you don\u0027t have keystone auth (or keycloak) then you are asking for trouble.\n\nRunning without keystone auth is a case specifically covered in the docs[1]. \"Mistral can be used without authentication at all or it can work with OpenStack.\"  The docs make this look like a supported case for production.\n\n\u003e Anyone could add workflows that could interact with any of your other actions and cause some harm.\n\nYes, it\u0027s true running without auth means someone could potentially interact with the other actions in nefarious ways, but the other actions don\u0027t necessarily allow code upload/execution.  IMO I think it\u0027s better to provide the information to intended users from the operators via some sort of documentation vs error messages in the system.  It is a basic security practice.\n\nAll I\u0027m saying is that if you run Mistral without auth and someone is poking at it via the web api, the current code would show valuable information on the first execution in the error message.  If the information is not divulged, the action would likely be executed multiple times, leaving more of a trail in logs.  An operator would likely provide the information on supported runtimes to intended users anyway.\n\nMost of the above concerns would be nullified if the code changes to supporting shebangs as the \"supported commands\" block would go away.\n\n[1] http://docs.openstack.org/developer/mistral/guides/configuration_guide.html","commit_id":"732f99d97f830b81e4b5f6cdc2ed68393751db2f"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"c0aedadcdd54cdbdb772ed69e53aa19b587a6a85","unresolved":false,"context_lines":[{"line_number":494,"context_line":"    def __init__(self, command, path, params\u003d[], exit_code\u003d0):"},{"line_number":495,"context_line":"        if command not in self.SUPPORTED_COMMANDS:"},{"line_number":496,"context_line":"            raise exc.ActionException("},{"line_number":497,"context_line":"                \u0027%s is not supported, current supported commands: %s\u0027 %"},{"line_number":498,"context_line":"                (command, self.SUPPORTED_COMMANDS)"},{"line_number":499,"context_line":"            )"},{"line_number":500,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5a8bc5a2_2aa048f4","line":497,"in_reply_to":"5a8bc5a2_4b8843e8","updated":"2016-09-22 13:28:24.000000000","message":"agree :-)","commit_id":"732f99d97f830b81e4b5f6cdc2ed68393751db2f"},{"author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"change_message_id":"a751277dc295bc0188499eb1ab8b819c347132b0","unresolved":false,"context_lines":[{"line_number":494,"context_line":"    def __init__(self, command, path, params\u003d[], exit_code\u003d0):"},{"line_number":495,"context_line":"        if command not in self.SUPPORTED_COMMANDS:"},{"line_number":496,"context_line":"            raise exc.ActionException("},{"line_number":497,"context_line":"                \u0027%s is not supported, current supported commands: %s\u0027 %"},{"line_number":498,"context_line":"                (command, self.SUPPORTED_COMMANDS)"},{"line_number":499,"context_line":"            )"},{"line_number":500,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5a8bc5a2_4b8843e8","line":497,"in_reply_to":"7a8ec9b2_2f419f63","updated":"2016-09-22 10:57:33.000000000","message":"I don\u0027t see a problem with revealing the commands. If you don\u0027t have keystone auth (or keycloak) then you are asking for trouble. Anyone could add workflows that could interact with any of your other actions and cause some harm.","commit_id":"732f99d97f830b81e4b5f6cdc2ed68393751db2f"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"01dd35bbdf620b3bb706ef6c1be2653e0f25db2e","unresolved":false,"context_lines":[{"line_number":494,"context_line":"    def __init__(self, command, path, params\u003d[], exit_code\u003d0):"},{"line_number":495,"context_line":"        if command not in self.SUPPORTED_COMMANDS:"},{"line_number":496,"context_line":"            raise exc.ActionException("},{"line_number":497,"context_line":"                \u0027%s is not supported, current supported commands: %s\u0027 %"},{"line_number":498,"context_line":"                (command, self.SUPPORTED_COMMANDS)"},{"line_number":499,"context_line":"            )"},{"line_number":500,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_2f419f63","line":497,"range":{"start_line":497,"start_character":38,"end_line":497,"end_character":64},"in_reply_to":"7a8ec9b2_7cde2012","updated":"2016-09-22 02:08:55.000000000","message":"If you run Mistral without keystone auth, it is already a suspicious behaviour isn\u0027t it? and I don\u0027t think exposing just supported commands will be vulneralbe to the cloud. We need to let end user know something he should know if error happens.","commit_id":"732f99d97f830b81e4b5f6cdc2ed68393751db2f"},{"author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"change_message_id":"d9a74fb0f83656800f15b086930290d7aaa34b15","unresolved":false,"context_lines":[{"line_number":495,"context_line":"        if command not in self.SUPPORTED_COMMANDS:"},{"line_number":496,"context_line":"            raise exc.ActionException("},{"line_number":497,"context_line":"                \u0027%s is not supported, current supported commands: %s\u0027 %"},{"line_number":498,"context_line":"                (command, self.SUPPORTED_COMMANDS)"},{"line_number":499,"context_line":"            )"},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"        if path not in CONF.executable_files:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_c83dc32c","line":498,"updated":"2016-09-21 11:40:42.000000000","message":"we don\u0027t need to show the full dictionary here. How about changing this to: \u0027, \u0027.join(self.SUPPORTED_COMMANDS)\n\nIt will only take the dictionary keys and display them nicely.","commit_id":"732f99d97f830b81e4b5f6cdc2ed68393751db2f"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"01dd35bbdf620b3bb706ef6c1be2653e0f25db2e","unresolved":false,"context_lines":[{"line_number":495,"context_line":"        if command not in self.SUPPORTED_COMMANDS:"},{"line_number":496,"context_line":"            raise exc.ActionException("},{"line_number":497,"context_line":"                \u0027%s is not supported, current supported commands: %s\u0027 %"},{"line_number":498,"context_line":"                (command, self.SUPPORTED_COMMANDS)"},{"line_number":499,"context_line":"            )"},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"        if path not in CONF.executable_files:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_6f2647cf","line":498,"in_reply_to":"7a8ec9b2_c83dc32c","updated":"2016-09-22 02:08:55.000000000","message":"Yeah, good suggestion, will fix that. Thanks!","commit_id":"732f99d97f830b81e4b5f6cdc2ed68393751db2f"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"c7aca0beb36a4337d7e54d0e279cfd3da2b336a4","unresolved":false,"context_lines":[{"line_number":495,"context_line":"        if command not in self.SUPPORTED_COMMANDS:"},{"line_number":496,"context_line":"            raise exc.ActionException("},{"line_number":497,"context_line":"                \u0027%s is not supported, current supported commands: %s\u0027 %"},{"line_number":498,"context_line":"                (command, self.SUPPORTED_COMMANDS)"},{"line_number":499,"context_line":"            )"},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"        if path not in CONF.executable_files:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_2371d776","line":498,"in_reply_to":"7a8ec9b2_c83dc32c","updated":"2016-09-21 15:18:08.000000000","message":"Yes, agree.","commit_id":"732f99d97f830b81e4b5f6cdc2ed68393751db2f"},{"author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"change_message_id":"d9a74fb0f83656800f15b086930290d7aaa34b15","unresolved":false,"context_lines":[{"line_number":520,"context_line":"            address_space\u003d1 * units.Gi,"},{"line_number":521,"context_line":"            number_files\u003d10,"},{"line_number":522,"context_line":"            number_processes\u003d5,"},{"line_number":523,"context_line":"        )"},{"line_number":524,"context_line":""},{"line_number":525,"context_line":"        try:"},{"line_number":526,"context_line":"            # Use os.setsid to create child process in a process group"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_e8e27f9f","line":523,"updated":"2016-09-21 11:40:42.000000000","message":"I don\u0027t like these arbitrary limits. I think they will be confusing for most users - why not make them all configurable now?","commit_id":"732f99d97f830b81e4b5f6cdc2ed68393751db2f"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"c0aedadcdd54cdbdb772ed69e53aa19b587a6a85","unresolved":false,"context_lines":[{"line_number":520,"context_line":"            address_space\u003d1 * units.Gi,"},{"line_number":521,"context_line":"            number_files\u003d10,"},{"line_number":522,"context_line":"            number_processes\u003d5,"},{"line_number":523,"context_line":"        )"},{"line_number":524,"context_line":""},{"line_number":525,"context_line":"        try:"},{"line_number":526,"context_line":"            # Use os.setsid to create child process in a process group"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a8bc5a2_8adddc72","line":523,"in_reply_to":"5a8bc5a2_6bb09f8c","updated":"2016-09-22 13:28:24.000000000","message":"Fair enough","commit_id":"732f99d97f830b81e4b5f6cdc2ed68393751db2f"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"341f07571aa198b076290e50eb030bf2a0696514","unresolved":false,"context_lines":[{"line_number":520,"context_line":"            address_space\u003d1 * units.Gi,"},{"line_number":521,"context_line":"            number_files\u003d10,"},{"line_number":522,"context_line":"            number_processes\u003d5,"},{"line_number":523,"context_line":"        )"},{"line_number":524,"context_line":""},{"line_number":525,"context_line":"        try:"},{"line_number":526,"context_line":"            # Use os.setsid to create child process in a process group"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a8bc5a2_f5781392","line":523,"in_reply_to":"5a8bc5a2_dc6264ad","updated":"2016-09-22 09:45:47.000000000","message":"Yeah, if most people here don\u0027t mind, I can certainly add more config items in subsequent patchset. (But what I heard from our ops was they are really tired of more and more options in configuration file :-) )","commit_id":"732f99d97f830b81e4b5f6cdc2ed68393751db2f"},{"author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"change_message_id":"a751277dc295bc0188499eb1ab8b819c347132b0","unresolved":false,"context_lines":[{"line_number":520,"context_line":"            address_space\u003d1 * units.Gi,"},{"line_number":521,"context_line":"            number_files\u003d10,"},{"line_number":522,"context_line":"            number_processes\u003d5,"},{"line_number":523,"context_line":"        )"},{"line_number":524,"context_line":""},{"line_number":525,"context_line":"        try:"},{"line_number":526,"context_line":"            # Use os.setsid to create child process in a process group"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a8bc5a2_6bb09f8c","line":523,"in_reply_to":"5a8bc5a2_f5781392","updated":"2016-09-22 10:57:33.000000000","message":"They can be configurable options to the task, but default to the values that you have provided (just like you did with exit_code). Most users can ignore them and use these defaults, I think they are okay as defaults but as hardcoded values I think they are too limiting.","commit_id":"732f99d97f830b81e4b5f6cdc2ed68393751db2f"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"01dd35bbdf620b3bb706ef6c1be2653e0f25db2e","unresolved":false,"context_lines":[{"line_number":520,"context_line":"            address_space\u003d1 * units.Gi,"},{"line_number":521,"context_line":"            number_files\u003d10,"},{"line_number":522,"context_line":"            number_processes\u003d5,"},{"line_number":523,"context_line":"        )"},{"line_number":524,"context_line":""},{"line_number":525,"context_line":"        try:"},{"line_number":526,"context_line":"            # Use os.setsid to create child process in a process group"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_fa74d79c","line":523,"in_reply_to":"7a8ec9b2_e8e27f9f","updated":"2016-09-22 02:08:55.000000000","message":"As I said in the comment, actually there are a lot of params that could be configured when using ProcessLimits. For a starting point, I just left several of those (they are what we care about most for now) hardcoded, I don\u0027t want to make everything configurable in the beginning, but we could definitely do that with more other use cases coming to us.","commit_id":"732f99d97f830b81e4b5f6cdc2ed68393751db2f"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"40b38205cb88a084787ee3f8f06efbe19c9caf71","unresolved":false,"context_lines":[{"line_number":520,"context_line":"            address_space\u003d1 * units.Gi,"},{"line_number":521,"context_line":"            number_files\u003d10,"},{"line_number":522,"context_line":"            number_processes\u003d5,"},{"line_number":523,"context_line":"        )"},{"line_number":524,"context_line":""},{"line_number":525,"context_line":"        try:"},{"line_number":526,"context_line":"            # Use os.setsid to create child process in a process group"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a8bc5a2_dc6264ad","line":523,"in_reply_to":"7a8ec9b2_fa74d79c","updated":"2016-09-22 09:12:36.000000000","message":"Maybe we should do more in the beginning? Keep in mind that once it\u0027s out people may start using it, it\u0027ll become a part of public API and will be difficult to change.","commit_id":"732f99d97f830b81e4b5f6cdc2ed68393751db2f"},{"author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"change_message_id":"40f8bd80f55aff2a428cfda79e6299ff701bd846","unresolved":false,"context_lines":[{"line_number":501,"context_line":"        for root, dirs, files in os.walk(CONF.scripts_dir):"},{"line_number":502,"context_line":"            if filename in files:"},{"line_number":503,"context_line":"                self.path \u003d os.path.join(root, filename)"},{"line_number":504,"context_line":"                break"},{"line_number":505,"context_line":"        else:"},{"line_number":506,"context_line":"            raise exc.ActionException("},{"line_number":507,"context_line":"                \u0027The script %s is not supported, please contact with your \u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_aebe35fd","line":504,"updated":"2016-09-22 11:15:59.000000000","message":"This should either use os.listdir() or, if we really want to support nested directories, it should check the full relative path.\n\nTake for example this directory structure:\n\n- directory1\n  - file.py\n- directory2\n  - file.py\n\nIf I pass in file.py it will pick one from directory1 or directory2, I don\u0027t know. I think it will not even be consistent.\n\nSo, users should either pass in \"directory1/file.py\" or we shouldn\u0027t allow sub-directories.","commit_id":"440706753cd9f96662e235215cb5683853972bcb"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"975cacf284e4de4dc342252c018d12a9db81fdf4","unresolved":false,"context_lines":[{"line_number":501,"context_line":"        for root, dirs, files in os.walk(CONF.scripts_dir):"},{"line_number":502,"context_line":"            if filename in files:"},{"line_number":503,"context_line":"                self.path \u003d os.path.join(root, filename)"},{"line_number":504,"context_line":"                break"},{"line_number":505,"context_line":"        else:"},{"line_number":506,"context_line":"            raise exc.ActionException("},{"line_number":507,"context_line":"                \u0027The script %s is not supported, please contact with your \u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_fe619da9","line":504,"in_reply_to":"5a8bc5a2_8d3fb624","updated":"2016-09-23 04:29:59.000000000","message":"As I said in my comment, we don\u0027t want same name files existed in the dir, it could be controlled by ops with appropriate guide. Because I think it is useful to have sub-dirs, so we have to make some compromise with each solutions here.\n\n\u0027not critical\u0027 I mean, even if user specify a filename with same name as others (configured erroneously by ops), the user can not get expected result, it\u0027s not harmful to the whole system. That can be quite easily solved by ops","commit_id":"440706753cd9f96662e235215cb5683853972bcb"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"caf684713a3218189e1d2bd5cc135aa7c27c4ad6","unresolved":false,"context_lines":[{"line_number":501,"context_line":"        for root, dirs, files in os.walk(CONF.scripts_dir):"},{"line_number":502,"context_line":"            if filename in files:"},{"line_number":503,"context_line":"                self.path \u003d os.path.join(root, filename)"},{"line_number":504,"context_line":"                break"},{"line_number":505,"context_line":"        else:"},{"line_number":506,"context_line":"            raise exc.ActionException("},{"line_number":507,"context_line":"                \u0027The script %s is not supported, please contact with your \u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_ca9364f9","line":504,"in_reply_to":"5a8bc5a2_aebe35fd","updated":"2016-09-22 13:37:53.000000000","message":"Yeah, there is a problem for files with same name. We should allow sub-dirs because sometimes scripts could be split into different dirs for different purposes. And I don\u0027t think let user input something like \u0027dirs/files.py\u0027 is a good idea, that means host folder tree could be revealed to users. It\u0027s also a little bit tricky to check name clash after configuration. So, the easiest way I can think of is, write down something in our doc as a reminder for ops. Anyway, even if there are same name files in different sub-dirs, it\u0027s not a critical issue for users.","commit_id":"440706753cd9f96662e235215cb5683853972bcb"},{"author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"change_message_id":"a16479e43d19a135cb789f3c615d95312c12b85f","unresolved":false,"context_lines":[{"line_number":501,"context_line":"        for root, dirs, files in os.walk(CONF.scripts_dir):"},{"line_number":502,"context_line":"            if filename in files:"},{"line_number":503,"context_line":"                self.path \u003d os.path.join(root, filename)"},{"line_number":504,"context_line":"                break"},{"line_number":505,"context_line":"        else:"},{"line_number":506,"context_line":"            raise exc.ActionException("},{"line_number":507,"context_line":"                \u0027The script %s is not supported, please contact with your \u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_8d3fb624","line":504,"in_reply_to":"5a8bc5a2_ca9364f9","updated":"2016-09-22 13:50:48.000000000","message":"\u003e Anyway, even if there are same name files in different sub-dirs, it\u0027s not a critical issue for users.\n\nSorry, but I do think that is a critical issue. There is no way to know which file you are calling if you are only passing the filename and not the path. That is just dangerous.","commit_id":"440706753cd9f96662e235215cb5683853972bcb"}]}
