gtristan commented on pull request #1436: URL: https://github.com/apache/buildstream/pull/1436#issuecomment-1047471821
I'm not particularly fond of this patch and how the `Platform` is digging quite deep into the `Sandbox` interfaces instead of letting `Sandbox` handle that bit, although this seems to already be a bit messy before the patch. Looking at this, my main concern is that we are no longer checking the sandbox config when creating a sandbox in `Element`, so this could cause some sort of regression in error reporting when actually trying to run commands in the sandbox where the config is not supported. It seems to me (and I might very well be missing something) that we need to have some semantic in `Element.__sandbox()` to dictate whether we need the sandbox for the purpose of *running commands* or not, in order to determine if we need to run this check or not. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
