Skip to content
This repository was archived by the owner on Aug 23, 2023. It is now read-only.

clean up more prometheus-ish dependencies #1614

Merged
merged 1 commit into from
Jan 20, 2020

Conversation

fitzoh
Copy link
Contributor

@fitzoh fitzoh commented Jan 17, 2020

More dependencies to remove from #1613 after 1fc250b.

Opening as a PR to a PR branch to keep diffs clean until someone can verify these are safe/sane to remove.

@fitzoh
Copy link
Contributor Author

fitzoh commented Jan 17, 2020

This passes the QA check and the base PR doesn't so I'm guessing this is probably safe (wasn't sure about github.com/golang/snappy)

@Dieterbe
Copy link
Contributor

Dieterbe commented Jan 18, 2020

why remove the master constraint on snappy?
github.com/gocql/gocql requires snappy. not sure if it really requires master though. either way, let's just focus on the prometheus related dependencies

@fitzoh
Copy link
Contributor Author

fitzoh commented Jan 19, 2020

why remove the master constraint on snappy?

#1613 is currently failing due to a QA check on vendor.sh:

Gopkg.lock is out of sync with imports and/or Gopkg.toml. Run `dep check` for details.
PROJECT  MISSING PACKAGES
input-digest mismatch

dep check listed snappy along with the prometheus dependencies.

Running dep ensure resulted in the following warnings:

$ dep ensure
Warning: the following project(s) have [[constraint]] stanzas in Gopkg.toml:

  ✗  github.com/golang/snappy
  ✗  github.com/prometheus/prometheus

However, these projects are not direct dependencies of the current project:
they are not imported in any .go files, nor are they in the 'required' list in
Gopkg.toml. Dep only applies [[constraint]] rules to direct dependencies, so
these rules will have no effect.

Either import/require packages from these projects so that they become direct
dependencies, or convert each [[constraint]] to an [[override]] to enforce rules
on these projects, if they happen to be transitive dependencies.

I can add it back, but dep seems to think it doesn't belong there

@Dieterbe
Copy link
Contributor

dep seems to think it doesn't belong there

it gives us two options. keep enforcing it by changing to an override, or removing it (in which case we will get a different version of snappy)

Generally, changing versions of dependencies (transitive or otherwise), should be done with care.
I'm in favor of keeping it simple and only have overrides/constraints if we know we need them. But it means we have to compare the two versions to see if we don't regress.

Strangely i'm not seeing an update to Gopkg.lock in your PR showing which version of snappy we would be pinning to now, but I assume it would be the latest version, v0.0.1
In that case, I've reviewed the changelog and it seems that version is equivalent to the version we were previously locked to.

@fitzoh
Copy link
Contributor Author

fitzoh commented Jan 19, 2020

it gives us two options. keep enforcing it by changing to an override, or removing it (in which case we will get a different version of snappy)

We would only get a new version if we dep ensure -update, right?

Strangely i'm not seeing an update to Gopkg.lock in your PR showing which version of snappy we would be pinning to now, but I assume it would be the latest version, v0.0.1
In that case, I've reviewed the changelog and it seems that version is equivalent to the version we were previously locked to.

Version present in the lockfile in both metrictank master and this PR:

[[projects]]
  branch = "master"
  digest = "1:9413ddbde906f91f062fda0dfa9a7cff43458cd1b2282c0fa25c61d89300b116"
  name = "github.com/golang/snappy"
  packages = ["."]
  pruneopts = "NUT"
  revision = "553a641470496b2327abcac10b36396bd98e45c9"

Resolved version of dep ensure -update from this PR branch:

[[projects]]
  digest = "1:7f114b78210bf5b75f307fc97cff293633c835bab1e0ea8a744a44b39c042dfe"
  name = "github.com/golang/snappy"
  packages = ["."]
  pruneopts = "NUT"
  revision = "2a8bb927dd31d8daada140a5d09578521ce5c36a"
  version = "v0.0.1"

Resolved version of dep ensure -update from master:

[[projects]]
  branch = "master"
  digest = "1:8cf1ded24c311b8675cd5c9a15bde6bff7a8132b8979dd881d8e58c7c2423a7a"
  name = "github.com/golang/snappy"
  packages = ["."]
  pruneopts = "NUT"
  revision = "ff6b7dc882cf4cfba7ee0b9f7dcc1ac096c554aa"

So as long as we don't dep ensure -update it doesn't really matter?

As you mentioned (Generally, changing versions of dependencies (transitive or otherwise), should be done with care.), we would have to do a bunch of testing when we updated anyways.

@Dieterbe
Copy link
Contributor

We would only get a new version if we dep ensure -update, right?

aha ok well that explains then why there is no update in Gopkg.lock. seems like that invalidates my concern. carry on.

@Dieterbe Dieterbe merged commit 81a6612 into remove-promql Jan 20, 2020
@Dieterbe Dieterbe deleted the remove-promql-more-dependencies branch January 20, 2020 18:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants