Skip to content

Add support for setting user labels on scheduled functions#3408

Merged
joehan merged 4 commits into
masterfrom
jh-functions-labels
May 26, 2021
Merged

Add support for setting user labels on scheduled functions#3408
joehan merged 4 commits into
masterfrom
jh-functions-labels

Conversation

@joehan

@joehan joehan commented May 26, 2021

Copy link
Copy Markdown
Member

Description

Adds support for user labels on scheduled functions.

Turns out we already supported user labels in this code path - except that they were being overwritten on scheduled functions.

Scenarios Tested

Confirmed that scheduled functions and non-scheduled functions are deployed with labels when firebase-functions passes them through.

@joehan joehan requested review from inlined and taeold May 26, 2021 18:28
@google-cla google-cla Bot added the cla: yes Manual indication that this has passed CLA. label May 26, 2021

@inlined inlined left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM with nit

}

cloudFunction.labels = { "deployment-scheduled": "true" };
cloudFunction.labels = Object.assign(cloudFunction.labels, {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: now that we have object destructuring I personally find it cleaner to use that syntax:

cloudfunctions.labels = {
   ...cloudfunctions.lables,
   "deployment-scheduled": true,
}

Or at least I would if we wanted to create a copy of the dictionary. In this case, your code is equivalent to:

cloudFunction.labels["deployment-scheduled"] = true;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I prefer that too, so I'll switch this over. As is, this is not quite equivalent to cloudFunction.labels["deployment-scheduled"], because labels is optional & can be undefined. Object.assign safely handles that case, wheres the above does not.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

> obj = {}
{}
> obj.labels = Object.assign(obj.lables, {"foo": "bar"});
Uncaught TypeError: Cannot convert undefined or null to object
    at Function.assign (<anonymous>)

Object splat supports undefined but Object.assign does not.

@joehan joehan merged commit a1f2e85 into master May 26, 2021
@joehan joehan deleted the jh-functions-labels branch May 26, 2021 20:47
devpeerapong pushed a commit to devpeerapong/firebase-tools that referenced this pull request Dec 14, 2021
…3408)

* add support for setting user labels on scheduled functions

* Style change to use destructuring, and CHANGELOG entry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Manual indication that this has passed CLA.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants