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

Formalize typedvalues implementation #218

Merged
merged 22 commits into from
Oct 3, 2018
Merged

Formalize typedvalues implementation #218

merged 22 commits into from
Oct 3, 2018

Conversation

erwinvaneyk
Copy link
Member

This change fixes long overdue issues with the typedvalues implementation. Due to initial prototyping and initial lack of clear context boundaries, this implementation has been fragile and the source of issues. Also, it's convoluted implementation made it difficult for external contributors to understand.

This PR solves this by redesigning the typedvalues package to be: (1) simple/low complexity (2) make the API less cryptic/more self-explanatory, and (3) more structured when it comes to HTTP parsing and formatting. Towards this end, this change...

  • switches the serialization underlying the typedvalues to use protobuf only, rather than a per-type difference.
  • centralizes all typedvalues-specific logic in the typedvalues package. Moving the protobuf declarations from a types package
  • Renamed labels on the typedvalues struct to metadata, to better illustrate its intended use.
  • Renamed parse/format of typedvalues to wrap/unwrap to better indicate the behaviour.
  • Extracted the parsers and formatters for HTTP requests to explicit interfaces in the httpconv package
  • Extracted all control logic related typedvalues to the controlflow package, in part to avoid a cyclic dependency between types and typedvalues.

Given that this formalization uncovers a lot of unhandled edge cases in the initial fragile implementation, several things were changed besides the typedvalues:

  • Updated the dependencies on kubernetes and Golang in Travis CI.
  • Added a check for the common error of using gogo-protobuf implementation instead of the golang-protobuf implementation.
  • Added explicit dependencies on testify and mergo.
  • Moved the environment proxy to a subpackage of the apiserver, instead of in the fnenv.
  • Renamed and restructured the apiserver api.

- Formalized the httpconv interface to explicit mappers. Supported mappers: text, bytes, json, protobuf, and form-data

Renamed Parse-Format to more intuitive Wrap-Unwrap analogy

- Implemented Flow as a Protobuf message
Needed because they depend on the types defined in valuetypes init function
We already depended on it, but never explicitely declared is as a
dependency. This also updates mergo to the latest version (3.6).
- Further ensured that empty requests/responses are handled properly.

- Refactored a couple of references to use the NPE-safe GetXXX
implementations.
Now both Fission and Workflows are more stable when it comes to deploying and calling functions near-instantly.
@erwinvaneyk erwinvaneyk merged commit 90cf2ae into master Oct 3, 2018
@erwinvaneyk erwinvaneyk deleted the typedvalues-v2 branch October 3, 2018 12:37
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

Successfully merging this pull request may close these issues.

1 participant