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

Speed up functions deployment by doing in-place trigger parsing #578

Merged
merged 4 commits into from Jan 5, 2018

Conversation

laurenzlong
Copy link
Contributor

@laurenzlong laurenzlong commented Dec 22, 2017

Description

Addresses #536.

Speeds up functions deployment:

This has been manually tested, deploy works as expected, and the uploaded source zip contains .runtimeconfig.json with the correct config values.

Sample Commands

No change to commands.

@coveralls
Copy link

coveralls commented Dec 22, 2017

Coverage Status

Coverage remained the same at 57.502% when pulling 873bd69 on ll-stopcopying into 4d63f5f on master.

@coveralls
Copy link

coveralls commented Dec 22, 2017

Coverage Status

Coverage remained the same at 57.502% when pulling ad6e284 on ll-stopcopying into 4d63f5f on master.

Copy link
Contributor

@mbleigh mbleigh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small thing, but otherwise this is great! 😄

return parseTriggers(getProjectId(options), tmpdir.name);
var configValues;
var sourceDir = options.config.path(options.config.get('functions.source'));
return _prepareSource(context).then(function(result) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's kinda weird that a function called prepareSource resolves with config values. Can we refactor this just a bit now that "preparing source" isn't copying things to a tmp dir?

@mbleigh mbleigh assigned laurenzlong and unassigned mbleigh Jan 2, 2018
@coveralls
Copy link

coveralls commented Jan 3, 2018

Coverage Status

Coverage remained the same at 57.502% when pulling f207915 on ll-stopcopying into 4d63f5f on master.

@laurenzlong
Copy link
Contributor Author

screenshot from 2018-01-04 11 35 06
This is what the error message looks like if someone has an outdated SDK

@mbleigh
Copy link
Contributor

mbleigh commented Jan 4, 2018

LGTM

@laurenzlong laurenzlong merged commit 40d2a76 into master Jan 5, 2018
@laurenzlong laurenzlong deleted the ll-stopcopying branch January 5, 2018 01:07
yuchenshi added a commit that referenced this pull request Aug 5, 2020
* Add delete and config emulator util APIs.

* Clean up typos and linter warnings.
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.

None yet

3 participants