Skip to content
This repository was archived by the owner on Dec 9, 2024. It is now read-only.

Fix for UpdateFunction #382

Merged
merged 2 commits into from
Mar 2, 2018
Merged

Fix for UpdateFunction #382

merged 2 commits into from
Mar 2, 2018

Conversation

alexdebrie
Copy link
Contributor

What did you implement:

Calling the UpdateFunction endpoint in the ConfigAPI updated the function in the default space, even if it was called for a different space. This PR fixes it.

How did you implement it:

The initial problem is in the UpdateFunction method in libkv package. It's passed a space and a *function.Function instance. The space is used correctly when checking for the function's existence, but then it uses the Space attribute on the function.Function instance when actually updating the function. The Space attribute has been set to "default" when it was sent through the validateFunction method.

I was going to just change the line to insert the updated function to use the space argument, but it seems like the different space confusion (space argument vs fn.Space) could happen again. Instead, I went up to the httpapi package and set the fn.Space there. This is similar to how it's done in the registerFunction handler and makes it easier to reason about.

The only change in behavior is that the function is now validated first, then checked for existence in the key-value store, rather than the other way around. This is because the validateFunction sets the function Space if it isn't already set, and I needed to do that before checking for existence in a given space. I don't think the switch of order matters.

The rest of the changes are just around changing the method signature, service interface, and tests for the new UpdateFunction method signature.

How can we verify it:

  1. Start the Event Gateway locally: go run cmd/event-gateway/main.go -dev

  2. Register a function in a space other than "default".

  3. Update the function.

  4. Call GetFunctions to ensure it reflects your update.

Todos:

  • Write tests
  • Write documentation
  • Fix linting errors
  • Make sure code coverage hasn't dropped
  • Provide verification commands / resources
  • Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

@alexdebrie alexdebrie requested a review from mthenw March 2, 2018 03:35
@mthenw mthenw merged commit d668383 into master Mar 2, 2018
@mthenw mthenw deleted the UpdateFunctionFix branch March 2, 2018 07:48
@mthenw mthenw changed the title EG-20 Fix for UpdateFunction Fix for UpdateFunction Mar 2, 2018
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants