Design: Path Resolver #48
Conversation
First pass of design to get comments from the community. Introduction of two pluggable components. * Path Resolver * Runtime Validator
There was a problem hiding this comment.
I am having a hard time understanding how (if at all) this impacts the consumer of the library. Will this change be transparent to callers trying to build a given lambda runtime? From the consumer side, can I pass the path to the executable to override the resolving (maybe the validator too) behavior?
How does this design scale? As we add more and more languages/runtimes, will we need to always update something here? Or because we are reading the PATH, as long the executable is available things will "magically" work?
How do we do the Runtime Validation? I am assuming this validation is dependent on the Runtime language specifically. If so, following a factory design might be better than having one class that handles all the validation.
|
@jfuss What I'm suggesting is that path resolution and validation be part of each workflow, and each workflow brings it own. We just a provide an interface to get an executable and a corresponding path resolver to the BaseWorkflow. This design should scale with every workflow, since it becomes the responsibility of the workflow. The issue with reading the PATH and magically working is where we sadly run into version conflicts :( The consumer of a workflow can bring their own Validator and path, we might have to rev up the json rpc protocol number and pass additional arguments to the Builder. Infact it may be necessary if the default workflow fails because of path resolution failing. In terms of users of the library getting impacted: #30 is an example of that. Some of this can also be mitigated by running the build portion in a container. |
|
|
||
| ### Open questions | ||
|
|
||
| * Should we constrain the interface for the PathResolver with an abstract base class? |
There was a problem hiding this comment.
What is the pros and cons about this? I think we need an defined interface, at the very least.
There was a problem hiding this comment.
Pros of this, is that any implementations will need to have appropriate methods defined, so that we dont run into funkiness at runtime. But yeah, I'm just proposing abc as a way to solidly lock down the interface.
| self.runtime = runtime | ||
| self.executables = [self.runtime, self.language] | ||
|
|
||
| def path(self): |
There was a problem hiding this comment.
Is the path allowed to be symlinks or should it be a fully resolved path?
There was a problem hiding this comment.
symlinks should be allowed in my opinion. (helps users have a quick fix to their path, with just changing symlinks)
There was a problem hiding this comment.
I guess my question is more, is the expectation of the path to be fully resolved or will that happen somewhere else?
There was a problem hiding this comment.
Path need not be fully resolved here, the chosen path will be exec'd by the validator to make a decision on the validity of the path, and if it matches the runtime.
|
I'm going to look to add Ruby to this after we merge the Ruby build logic. Basically, the following shell one-liner will check compatibility with the ruby -e "unless RUBY_VERSION.match(/2\.5\.\d/); exit(1); end" |
First pass of design to get comments from the community.
Introduction of two pluggable components.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.