GoogleCloudPlatform / nodejs-docs-samples

Node.js samples for Google Cloud Platform products.
https://cloud.google.com/nodejs
Apache License 2.0
2.79k stars 1.93k forks source link

Decrease Renovate Frequency #1993

Closed grant closed 3 years ago

grant commented 3 years ago

Hey repo folks 👋, I'm very thankful we have bots automating some mundane tasks like updating dependency semvers. However, the frequency of these updates is causing micro delays (a minute a week maybe) and not very beneficial for the health of our system.

Suggestion

To increase the usefulness of this bot, and be considerate of our time, I suggest we change the frequency of Renovate (renovate.json) to once a quarter:

{
  "schedule": ["every 3 months on the first day of the month"]
}

https://docs.renovatebot.com/configuration-options/#schedule

That way we can ensure we have healthy dependencies and reduce GitHub notifications. At least let's try it for a bit?


@JustinBeckwith @ace-n

Original PR: https://github.com/GoogleCloudPlatform/nodejs-docs-samples/pull/778

ace-n commented 3 years ago

+1000 - though maybe we should do it a bit more frequently, e.g. every 1-2 months?

JustinBeckwith commented 3 years ago

I think the node.js ecosystem just moves too fast to make a 1 quarter timeframe viable.

I'm not opposed to some sort of batching of the requests, but I wouldn't hold these up for more than a week, let alone 3 months. I want to learn more about this:

However, the frequency of these updates is causing micro delays (a minute a week maybe)

Can you explain that a bit? How is the current setup exactly causing pain?

ace-n commented 3 years ago

I can't speak for @grant here, but personally - these notifications usually require some amount context-switching. (Not that they always do, since GitHub doesn't require each product owner to sign off on these updates [though I wish it did].)

If I can approve them in a batch (or better yet, at some defined time(s) within the week/month/year), that's O(1) context switches - if I can't, then it's O(n).

grant commented 3 years ago

Perhaps it's my personal notification settings, but this repo, rather than other languages/repos produces a lot more noise that doesn't seem to benefit. I know Node has a fairly unique ecosystem with deps.

For example, these recent PRs don't strongly benefit the products I work on like functions/, run/:

I'd rather spend one day a quarter updating dependencies (one by one if things are really out of date).

Perhaps I just need to approve then unsubscribe from them quickly to stop notifications.

ace-n commented 3 years ago

Another idea - @JustinBeckwith if your team is on the hook for making sure these won't break samples (or rolling them back automatically + pinging us product owners if they do), then maybe individual product owners can ignore these as Grant suggested.

bcoe commented 3 years ago

I wanted to add some perspective, from one layer above the samples...

Once folks take sample code, and integrate it into their applications, they quickly become hyper-aware of out-of-date dependencies. Services like Snyk and npm audit display frightening notifications, if dependencies are stale:

https://www.npmjs.com/advisories/1561

Here's a customer issue from today. The user is getting notified of a high severity security vulnerability, and can't take a patch for it, as they're on @google-cloud/speech@3.x.x and the latest version of @google-cloud/speech@4.x.x.

The client libraries team makes an effort to only update major versions once or twice a year (frequent upgrades are a pain for both us and customers.). But, it's very important when we choose to make these major bumps,. that they get out to customers.

Edit: an important part of getting these major bumps out to customers is that our samples demonstrate their usage.

bcoe commented 3 years ago

if your team is on the hook for making sure these won't break samples (or rolling them back automatically + pinging us product owners if they do)

@ace-n we make an effort to ensure that there are not breaking changes in client libraries, and to ensure that these breaking changes happen infrequently. Speaking for Node.js, we make an effort to only bump a major yearly.

There are, however, sometimes extenuating circumstances, e.g., a client library was released for a beta API, and the product has now released a v1 version of the upstream API, which has slightly different signature.


In the case of yearly updates, we use this as an opportunity to move best practices forward, e.g., dropping EOL versions of the platform.

In the case of breaking API changes, we attempt to work with partner teams to make these rare, and intentional, but they inevitably come up.

grant commented 3 years ago

Maybe we can auto-merge devDeps? Or automerge PRs that succeed with a set of requiredStatusChecks?

"packageRules": [
    {
      "depTypeList": ["devDependencies"],
      "automerge": true
    }
  ]

I'm just not providing much value to these PRs: https://github.com/GoogleCloudPlatform/nodejs-docs-samples/pulls?q=is%3Apr+author%3Arenovate-bot+reviewed-by%3Agrant

@bcoe In terms of the security anecdote, I don't think Renovate aims to solve that with the async nature. I would assume GitHub Dependabot alerts aim to solve that rather than Renovate. Of course, any mechanism to update deps helps.

bcoe commented 3 years ago

Maybe we can auto-merge devDeps? Or automerge PRs that succeed with a set of requiredStatusChecks?

If the devDeps are for our eyes only, i.e., not in the samples that we actually embed for customers, I feel less strongly about frequent updates being necessary.

My only concern would be that it does leave us slightly more open to security vulnerabilities. Dependabot seems to be doing a good job of raising security issues, but it's new to our workflow comparatively to renovate (so I just don't have the same level of trust yet).

_Edit: I've had a few conversations with OSPO about auto-merging behavior, around GitHub actions that have permission to merge. As it stands today, there are security and privacy concerns related to having an external entity submit code that's automatically merged (unfortunately, as it could improve some workflows). We would need to figure this out with OSPO._

fhinkel commented 3 years ago

I'm stronlgy in favor of updating dependencies as soon as there are updates available. @grant maybe you need to change your notification settings.