alexellis / derek

Reduce maintainer fatigue by automating GitHub
https://github.com/alexellis/derek/blob/master/USER_GUIDE.md
MIT License
808 stars 70 forks source link

add handler package #100

Closed cheikhshift closed 5 years ago

cheikhshift commented 5 years ago

Move source code suffixed with Handler to package handler.

Description

Add a new sub-level package.

Motivation and Context

It creates a new package to isolate derek api functionality. This is to move closer towards deploying derek as a function.

How Has This Been Tested?

I refactored a local instance of the project to use package handler. I also added a local FaaS function to test the Handler package with.

I used faas-cli 0.7.7 to build and deploy the function.

This will require updating main.go to use the handler package.

Types of changes

Checklist:

derek[bot] commented 5 years ago

Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off. That's something we need before your Pull Request can be merged. Please see our contributing guide.

alexellis commented 5 years ago

Thanks for this PR. Looks like the 2nd commit is missing the signed-off-by line/flag.

Also think you'll need to update references in the main.go to use the new handler package.

For example

handleComment needs to become HandleComment so it is visible outside the handler package via the main package such as handler.HandleComment().

I know I said about moving the these files in my comment on the issue, but I meant in a way that works/compiles/passes the build.

cheikhshift commented 5 years ago

Sure.

alexellis commented 5 years ago

I'm heading off for today. It would be good to get the commits squashed down into one before review. Thanks for getting involved :+1:

cheikhshift commented 5 years ago

Sure.

Happy to help.

On Oct 27, 2018, at 9:31 PM, Alex Ellis notifications@github.com wrote:

I'm heading off for today. It would be good to get the commits squashed down into one before review. Thanks for getting involved 👍

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

alexellis commented 5 years ago

@rgee0 I think this can be merged, do you have any input?

alexellis commented 5 years ago

@cheikhshift I think for the next part this unfortunately means the function will vendor the same GitHub repo it lives in which makes it a bit of a challenge to test/release changes. Any other ideas for this?

cheikhshift commented 5 years ago

None as of now.

The quick solution I used during development was to have a separate vendor folder within the function's folder, this ensured build success and passed tests. I'll learn more about the FaaS build command to see if there is a better/reliable way.

cheikhshift commented 5 years ago

Would it be possible to add a function sub package to this repository?

As a solution to get around updating function dependencies, I wrote a bash script to handle building and deploying the function. This will require updating the function handler to be this new sub package. The package will have a file named handler.go, it will be a copy of ./main.go, with function main updated to Handle(requestRaw []byte) string (This script is assuming the package will named fn)

I also think that the bash script solution is a bit hacky...

#!/bin/sh

if [ ! -d "./fn/vendor" ]; then

cd ./fn

dep init -gopath

cd ../

fi

faas-cli build -f ./derek.yml && \

faas-cli deploy -f ./derek.yml

rm -rf ./fn/vendor

rm ./fn/Gopkg.*