Skip to content
This repository has been archived by the owner on May 1, 2020. It is now read-only.

Add support for custom method invocation on a template file. #8

Merged
merged 3 commits into from
Oct 21, 2019
Merged

Add support for custom method invocation on a template file. #8

merged 3 commits into from
Oct 21, 2019

Conversation

Pavel910
Copy link
Contributor

This PR attempts to solve the problem discussed in issue #497.

This is my take on the problem, @eahefnawy please see the explanation below and let me know if you have a different idea about the implementation:

Solution

Everything is handled by the Template component, so from cli perspective nothing is changed.
The cool part is, when cli checks if the component has a custom method, and the component happens to be an instance of Template, the Proxy will return a "proxy" function to execute custom methods . Parameters --component are located in the inputs, so we can easily treat them equally from both CLI and programmatic perspective.

Below is the example invocation and the resulting output:
image

Let me know what you think and if any changes are required 🍻

@Pavel910 Pavel910 changed the title Add support for custom method invocation on a template file. WIP: Add support for custom method invocation on a template file. Oct 13, 2019
@Pavel910 Pavel910 changed the title WIP: Add support for custom method invocation on a template file. Add support for custom method invocation on a template file. Oct 13, 2019
@eahefnawy
Copy link
Contributor

@Pavel910 that's an interesting approach! I like that everything is handled by the template as this is where it should be.

Parameters --component are located in the inputs

If that's the case, why is the change serverless/components#509 required? 🤔 ... Aren't all inputs in the first object argument?

@Pavel910
Copy link
Contributor Author

@eahefnawy the problem with that PR is not the inputs, but access to component's state.
Once I load the relevant component and run the custom method, I would like to be able to access the current state of the component, but that doesn't really work, because I'm loading component via instance of the Template, here is the reference, and that triggers your wrapper function which ignores all but the first parameter. That causes the alias to not be passed in, and the component state doesn't get loaded. So that other PR fixes argument handling.

@eahefnawy eahefnawy merged commit 190adf2 into serverless:master Oct 21, 2019
@eahefnawy
Copy link
Contributor

Thanks @Pavel910 ... that makes sense!

I love how you can now run a single command on multiple components. When I first thought of this, I only considered running a single command on a single component.

The only thing I'm worried about is that we haven't really defined the components CLI as a whole. (what commands & options..etc). So may need to set this install or --component command/option as reserved if we need to. But for now, it's super flexible.

LG2M! 🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants