Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

template and variable resolution incomplete / mixed with usage #681

Closed
erikerikson opened this issue Feb 19, 2016 · 7 comments
Closed

template and variable resolution incomplete / mixed with usage #681

erikerikson opened this issue Feb 19, 2016 · 7 comments
Milestone

Comments

@erikerikson
Copy link
Member

My s-function.json is valid JSON:

"$${functionTemplate}"

The template is not resolved so that the contents fail to be valid s-function.json contents despite the template resolving to those. (I get an erroneous and misleading "function does not exist" message [see ERRONEOUS_FUNCTION_NOT_EXISTS below for stack trace])

The reason is that the evaluation of variables and templates is done on a case by case basis in the code instead of globally as part of loading the file. I assert that all templates and then all variables should be resolved before those files are considered loaded. This is not the case and the consequences are greater complexity in the Serverless code base and spotty/hidden support for templates and variable (or not) that generates wasteful confusion in the community.

The current implementation violates the default assumptions of those using templates and variables. I say this because of my own assumptions but also the repeated questions about why they aren't working in the gitter channel.

ERRONEOUS_FUNCTION_NOT_EXISTS
/usr/local/lib/node_modules/serverless/node_modules/bluebird/js/release/async.js:48
fn = function () { throw arg; };
^

ServerlessError: Function could not be loaded because it does not exist in your project: customers/undefined
at new ServerlessError (/usr/local/lib/node_modules/serverless/lib/ServerlessError.js:17:11)
at /usr/local/lib/node_modules/serverless/lib/ServerlessFunction.js:129:17
From previous event:
at ServerlessFunction.load (/usr/local/lib/node_modules/serverless/lib/ServerlessFunction.js:122:25)
at loadFn (/usr/local/lib/node_modules/serverless/lib/ServerlessComponent.js:80:25)
at /usr/local/lib/node_modules/serverless/lib/ServerlessComponent.js:113:28
From previous event:
at ServerlessComponent.load (/usr/local/lib/node_modules/serverless/lib/ServerlessComponent.js:102:14)
at /usr/local/lib/node_modules/serverless/lib/ServerlessProject.js:84:28
at processImmediate as _immediateCallback
From previous event:
at ServerlessProject.load (/usr/local/lib/node_modules/serverless/lib/ServerlessProject.js:69:8)
at ServerlessState.load (/usr/local/lib/node_modules/serverless/lib/ServerlessState.js:37:26)
at Serverless.init (/usr/local/lib/node_modules/serverless/lib/Serverless.js:112:23)
at /usr/local/lib/node_modules/serverless/lib/Serverless.js:142:56
From previous event:
at Serverless._execute (/usr/local/lib/node_modules/serverless/lib/Serverless.js:131:38)
at Serverless.actions.(anonymous function) (/usr/local/lib/node_modules/serverless/lib/Serverless.js:381:20)
at Serverless.command (/usr/local/lib/node_modules/serverless/lib/Serverless.js:350:38)
at Object. (/usr/local/lib/node_modules/serverless/bin/serverless:16:12)
at Module._compile (module.js:435:26)
at Object.Module._extensions..js (module.js:442:10)
at Module.load (module.js:356:32)
at Function.Module._load (module.js:311:12)
at Function.Module.runMain (module.js:467:10)
at startup (node.js:136:18)
at node.js:963:3

@erikerikson
Copy link
Member Author

An example is that within s-function.json:

  1. "endpoints" can contain variables but "events" cannot
  2. "customName" can contain variables but "customRole" cannot

@erikerikson
Copy link
Member Author

There's been discussion of changing from the format
"${variable} $${template}"
to
"$V{variable} $T{template}"

To maintain backwards compatibility, the old "${}$${}" formats could imply single pass template and variable replacement while the new formats can accept recursive templating and variables. Basically, while there is a template or variable in the the input, replace them with the stringified values associated with the declared template/variable in the variables and template files. When none are left, parse the resulting JSON as the user's intended configuration file. The raw version could still be made available.

The problem here is if you want to modify the original file programmatically because how do you determine and correctly modify origin validly for its applied scope? What are the use cases for such programmatic update of the static resources? I currently suspect there's not a ton of value in those use cases but don't want to assert there couldn't be any.

@erikerikson
Copy link
Member Author

Sorry for dunderheadedness and slow followup. The template and variable replacements need to happen in different ways. Templates happen immediately while variables are context sensitive. This means that the grounding operation would need to occur at load time for templates and use time for variables. A clear separation of the logic using the final values and producing the final values remains desirable.

@eahefnawy
Copy link
Member

Really sorry for the late reply @erikerikson, I was finding it a bit hard to decode the issue :) ... but generally speaking this is an interesting attempt to use templates. We never thought about putting the entire s-function.json in a template, and the way we designed our Utils.populate function doesn't handle this case, simple because we look into each value of the JSON properties without looking at the keys (checkout this line). So referencing variables and templates has to be in values only, not keys. So putting only "$${functionTemplate}" in s-function.json would obviously cause the populate function to fail.

Fixing this issue would require a complete redesign of how we handle variables and templates like you suggested, this however will require further discussion, and I don't think it'll be covered in v0.5.

@erikerikson
Copy link
Member Author

@eahefnawy sorry it was a bit cryptic. I think you're right to put off a fix. I support not delaying v0.5. I strongly support the suggested fix in the near term, I say to noone's surprise. :D

I'll be opening up another issue resulting from this same source. It too can probably be ignored for the short term.

To be concrete, the following snippets demonstrate the potential difference (between the s-template and s-function - all addition is repetitive across every function in my API) in size of representation:

s-template:

{
  "functionName": "[NAME]",
  "endpointsTemplate": [
    "$${endpointsPostTemplate}"
  ],
  "requestParametersTemplate": {
    [...]
  },
  "requestTemplatesTemplate": {
    "application/json": {
      [...]
    }
  }
}

s-function:

{
  "name":       "[NAME]",
  "customName": "$${customNameTemplate}",
  "customRole": "$${customRoleTemplate}",
  "handler":    "$${handlerTemplate}",
  "timeout":    "$${timeoutTemplate}",
  "memorySize": "$${memorySizeTemplate}",
  "custom":     "$${customTemplate}",
  "endpoints":  [
    {
      "path":               "$${pathTemplate}",
      "method":             "POST",
      "type":               "$${typeTemplate}",
      "authorizationType":  "$${authorizationTypeTemplate}",
      "apiKeyRequired":     "$${apiKeyRequiredTemplate}",
      "requestParameters": {
        [...]
      },
      "requestTemplates": {
        "application/json": {
          "application/json": {
            [...]
          }
        }
      },
      "responses":          "$${responsesPostTemplate}"
    }
  ],
  "events": "$${eventsTemplate}",
  "vpc": "$${vpcTemplate}",
  "runtime": "$${runtimeTemplate}"
}

This written/shown, there is somewhat of a cost to the brevity. I had to depend on my knowledge of how the templates combined across the files. As written, I think all the templating could be moved to the s-project > s-function files.

@flomotlik
Copy link
Contributor

@erikerikson agreed this needs to be resolved so that we have proper variable resolution once for the whole file. It impacts V1 as we're removing those configuration files and setting them up in a different, way, but the idea still applies.

@flomotlik
Copy link
Contributor

@erikerikson this should be resolved in V1 now as variable resolution happens once per serverless.yml file. I'll close this issue, if it isn't resolved in your opinion let me know and I'll reopen.

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

No branches or pull requests

5 participants