fix: multiple issues with gofalcon example and client opts #38
Conversation
|
👋 @jsteenb2 PTAL and let's chat on Slack if you have questions. |
| @@ -161,23 +161,19 @@ import ( | |||
| "log/slog" | |||
|
|
|||
| fdk "github.com/CrowdStrike/foundry-fn-go" | |||
| "github.com/CrowdStrike/gofalcon/falcon" | |||
| "github.com/CrowdStrike/gofalcon/falcon/client" | |||
| "github.com/crowdstrike/gofalcon/falcon" | |||
There was a problem hiding this comment.
this is very interesting, why does it need to be lowercase for gofalcon but not foundry-fn-go? Its the same github org yah?
There was a problem hiding this comment.
The difference is from the go.mod - it's lowercase in gofalcon but capital CS in foundry-fn-go.
There was a problem hiding this comment.
super weird, curious why that's the case 🤔
There was a problem hiding this comment.
AFAIK all lower is the convention
- "By convention, packages are given lower case, single-word names", https://go.dev/doc/effective_go#names
- "Good package names are short and clear. They are lower case, with no under_scores or mixedCaps", https://go.dev/blog/package-names
- And just looking at e.g. https://github.com/CrowdStrike/gofalcon/blob/main/go.mod
There was a problem hiding this comment.
I know we've had users say they have to use the mixed case for go get, or it fails, which is super annoying. Ideally it'd be all lowercase. I'll have to figure out what's going on there.
| @@ -19,15 +19,14 @@ var version = "development" | |||
| // AccessToken: token, | |||
| // Cloud: falcon.Cloud(opts.Cloud), | |||
| // Context: ctx, | |||
| // UserAgentOverride: out.UserAgent, | |||
| // UserAgentOverride: opts.UserAgent, | |||
README.md
Outdated
| return fdk.Response{ /* snip */ } | ||
| } | ||
| // some other error - see gofalcon documentation | ||
| return fdk.Response{ Code: 500, Body: fdk.JSON(err) } |
There was a problem hiding this comment.
What's the motivation for returning a jsonified err? I think we're better off sticking with the fdk.ErrResp(fdk.APIError{Code: 500, Message: err.Error()}. This ties into fusion's execution log UI as well as align with the idiomatic error handling of errors in a funciton.
There was a problem hiding this comment.
Ack hadn't seen this, will update.
There was a problem hiding this comment.
unfortunately, looking at the sample apps for foundry, we don't have funcitons that utilize even a small percentage of our best practices. They need some TLC, so things like fdk.ErrResp, fdk.HandleFnOf, logging, etc are in plain view
This PR addresses multiple issues identified while developing a Falcon Function in Golang with gofalcon:
importfalcon.ErrFalconNoTokenoutwhich is incorrectly copied fromFalconClientOpts()Also corrects minor unrelated Markdown formatting.