Better Functions initialization #3507
Conversation
| private readonly module: Module | ||
| ) {} | ||
| validate(): Promise<void> { | ||
| // throw new FirebaseError("Cannot yet analyze Go source code"); |
There was a problem hiding this comment.
Remove this commented code or switch it to a logger.debug statement?
| cloudFunctions: [ | ||
| { | ||
| apiVersion: 1, | ||
| id: "HelloWorld", |
There was a problem hiding this comment.
It looks like this function is in a demo state and you haven't fully implemented pulling some of these values from the Go code (unless it happens later?). I'm ok with leaving it like this for now, but a TODO comment would be helpful.
There was a problem hiding this comment.
Switched to throwing. I no longer need this to help ensure the setup works (it doesn't create a viable project as written actually, but I'll fix that in the followup that finishes e2e go deployment)
|
|
||
| /** Supported runtimes for new Cloud Functions. */ | ||
| const RUNTIMES: string[] = ["nodejs10", "nodejs12", "nodejs14"]; | ||
| const RUNTIMES: string[] = ["nodejs10", "nodejs12", "nodejs14", "go113"]; |
There was a problem hiding this comment.
Will go113 show up in any error messages for users who haven't --open-sesamed? If so, we might want to move it to an EXPERIMENTAL_RUNTIMES type so that we can hide it a bit better.
There was a problem hiding this comment.
There are no print statements that use RUNTIMES, but I've created the second list as a defense in depth.
| return; | ||
| } | ||
|
|
||
| // Go will refuse to overwrite an existing mod file. |
There was a problem hiding this comment.
Should we just copy go's behavior and error here?
There was a problem hiding this comment.
TypeScript and JavaScript do a prompt to overwrite and we do above here. That seems correct to me.
| const extractArchive = unzipper.Extract({ path: config.path("functions/firebase-functions-go") }); | ||
| await promisify(stream.pipeline)(download.body, extractArchive); | ||
|
|
||
| // Should this come later as "would you like to install dependencies" to mirror Node? |
There was a problem hiding this comment.
I think it makes sense to just always go get instead of prompting like we do for node. If I was developing, I'd prefer that, and I asked a few other peeps who have developed in go which they would expect, and they agreed.
There was a problem hiding this comment.
Sounds good. Removed comment.
This change adds support for `firebase --open-sesame golang`. After running this command, `firebase init` will support Go 1.13 as a langauge for Cloud Functions. Limitations: 1. .gitignore is empty 2. Customers cannot mix Node and Go code (WAI) 3. There is little validation being done of customer code 4. The actual deployed function params are hard coded; SDK incoming
inlined
force-pushed
the
inlined.create-functions
branch
2 times, most recently
from
1b90501 to
6ea031c
Compare
|
This is wicked 🤘 |
|
@jthegedus We decided to improve development velocity by developing in the open, but this is definitely not something we're hoping to advertise. Please don't share this broadly or we'll have to move development into a private repo. We'll keep you in mind when it's time to onboard alpha testers though! |
| } | ||
|
|
||
| // A module can be much more complicated than this, but this is all we need so far. | ||
| // for a full reference, see https://golang.org/doc/modules/gomod-ref |
There was a problem hiding this comment.
| // for a full reference, see https://golang.org/doc/modules/gomod-ref | |
| // For a full reference, see https://golang.org/doc/modules/gomod-ref |
| } | ||
| } | ||
|
|
||
| return module; |
There was a problem hiding this comment.
Should this error on cases where module.module or module.version doesn't get defined?
|
|
||
| constructor( | ||
| private readonly projectId: string, | ||
| private readonly sourceDirName: string, |
There was a problem hiding this comment.
I don't love that we have both sourceDirName and sourceDir - its a bit unclear at first what each means, it seems like you can always infer sourceDirName from sourceDir.
There was a problem hiding this comment.
You can derive sourceDirName from projectDir and sourceDir. I've removed sourceDirName from Go where it was unused and have made it derived in Node
Yes of course! 🤐 Keep me in mind for FuncsV2 alpha please 🙏 |
|
@jthegedus I followed you on Twitter. DM me your email associated with your GCP projects and I'll see what I can do to get you onboarded when we're ready. |
Basic create support This change adds support for `firebase --open-sesame golang`. After running this command, `firebase init` will support Go 1.13 as a langauge for Cloud Functions. Limitations: 1. Customers cannot mix Node and Go code (WAI) 2. There is little validation being done of customer code 3. The actual deployed function params are hard coded; SDK incoming
3 participants
Adds the firebase init workflow for Go functions. This is the first checkpoint I took in the hackathon. The next PR fleshes out the sample code and gitignore a bit as well as adds support for HTTP contract discovery. I'll try to get that one ready by tonight.
To try out the feature, run
firebase --open-sesame golang