Skip to content

Conversation

@lina128
Copy link
Collaborator

@lina128 lina128 commented Jan 3, 2020

and also add success notification.

Note: The function is manually changed and tested in gcp's UI and is working well. This PR is to keep the function in sync.


This change is Reviewable

@lina128 lina128 requested a review from dsmilkov January 3, 2020 21:44
Copy link
Contributor

@dsmilkov dsmilkov left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @dsmilkov and @lina128)


tfjs-core/scripts/cloud_funcs/send_email/index.js, line 34 at r1 (raw file):

  const build = JSON.parse(new Buffer(event.data, 'base64').toString());
  // Also added 'SUCCESS' to monitor successful builds.
  const status = [

we decided to ignore successful nightly builds since they are not actionable and help reduce email clutter. WDYT?

@lina128
Copy link
Collaborator Author

lina128 commented Jan 3, 2020

Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @dsmilkov and @lina128)

tfjs-core/scripts/cloud_funcs/send_email/index.js, line 34 at r1 (raw file):

  const build = JSON.parse(new Buffer(event.data, 'base64').toString());
  // Also added 'SUCCESS' to monitor successful builds.
  const status = [

we decided to ignore successful nightly builds since they are not actionable and help reduce email clutter. WDYT?

Ping asked to add the success case so that at least we receive the notification daily, otherwise we don't know if the notification itself is working. Personally, I don't have a strong opinion on either idea. Both sounds reasonable :)

Copy link
Contributor

@dsmilkov dsmilkov left a comment

Choose a reason for hiding this comment

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

That makes sense too. LGTM . We can setup email filters

@lina128 lina128 merged commit 7ba9a7e into tensorflow:master Jan 4, 2020
@lina128 lina128 deleted the cloud-func branch April 14, 2020 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants