Circular dependency with client_golang #58
Comments
|
In Golang dependencies are defined on a package level, and not on On Sat, Sep 10, 2016 at 3:19 PM Martín Ferrari notifications@github.com
|
|
Grobie, Yes, this is circular only in the repository sense, which is the one I care about :-) Now, this is not a critical bug, but it means more pain for users and release managers, with no real benefit, so I would like to fix it. Why do you say that after merging client_golang and common I will keep encountering these problems? |
|
Please note that I would really like not to do this hack, as it is also a lot more trouble for me.. |
|
`common` is a bag of commonly used packages. Previously, some of it
was part of `client_golang`, which was back then an even bigger bag
with a misleading name on top. (Now it is essentially two packages,
the exposition library and the API client.)
Avoiding circular dependencies on the repository level will be hard.
In terms of deb packaging, I'd suggest to either do the more detailed
approach and mirror the Go packages in `common` (and perhaps partially
`client_golang`), i.e. let Go enforce that there will be no circular
dependencies. Or you become coarser and put `client_golang` and
`common` all into one deb package.
|
|
@TheTincho I meant that without mirroring the go package structure you'll continue to run into such problems with the current common repository. I'm not sure merging common into client_golang is something we want to do, the packages are also used without client_golang. |
|
I don't really see us addressing this issue and I'm tempted to close it as "not feasible". @TheTincho how are you dealing with this at the moment? |
|
At the moment, I am able to work around this by excluding common/version from build and tests, and then adding it manually to the package. This way, at least I don't get a circular build dependency (which would give all kinds of headache for bootstrapping, backporting, etc), but the resulting packages still have the circular dep. So, it is not ideal, but not as bad. |
|
OK, thanks. I'll close this then because realistically, I don't see us changing the repo structure, given that a workaround for Debian is in place. |
03cc598 Don't lint generated protobuf code. 2b55c2d Merge pull request prometheus#66 from weaveworks/reduce-test-timeout d4e163c Make timeout a flag 49a8609 Reduce test timeout 8fa15cb Merge pull request prometheus#63 from weaveworks/test-defaults b783528 Tweak test script so it can be run on a mca a3b18bf Merge pull request prometheus#65 from weaveworks/fix-integration-tests ecb5602 Fix integration tests f9dcbf6 ... without tab (clearly not my day) a6215c3 Add break I forgot 0e6832d Remove incorrectly added tab eb26c68 Merge pull request prometheus#64 from weaveworks/remove-test-package-linting f088e83 Review feedback 2c6e83e Remove test package linting 2b3a1bb Merge pull request prometheus#62 from weaveworks/revert-61-test-defaults 8c3883a Revert "Make no-go-get the default, and don't assume -tags netgo" e75c226 Fix bug in GC of firewall rules. e49754e Merge pull request prometheus#51 from weaveworks/gc-firewall-rules 191f487 Add flag to enale/disable firewall rules' GC. 567905c Add GC of firewall rules for weave-net-tests to scheduler. 03119e1 Fix typo in GC of firewall rules. bbe3844 Fix regular expression for firewall rules. c5c23ce Pre-change refactoring: splitted gc_project function into smaller methods for better readability. ed5529f GC firewall rules ed8e757 Merge pull request prometheus#61 from weaveworks/test-defaults 57856e6 Merge pull request prometheus#56 from weaveworks/remove-wcloud dd5f3e6 Add -p flag to test, run test in parallel 62f6f94 Make no-go-get the default, and don't assume -tags netgo 8946588 Merge pull request prometheus#60 from weaveworks/2647-gc-weave-net-tests 4085df9 Scheduler now also garbage-collects VMs from weave-net-tests. 4b7d5c6 Merge pull request prometheus#59 from weaveworks/57-fix-lint-properly b7f0e69 Merge pull request prometheus#58 from weaveworks/fix-lint 794702c Pin version of shfmt ab1b11d Fix lint d1a5e46 Remove wcloud cli tool 81d80f3 Merge pull request prometheus#55 from weaveworks/lint-tf 05ad5f2 Review feedback 4c0d046 Use hclfmt to lint terraform. git-subtree-dir: tools git-subtree-split: 03cc5989769d93aa03f8aed3784ddfdb1fffc1c6
* dep ensure * Replace type with struct * Add instantiation test for HistogramCollector
|
I just want to be clear, the proposed workaround does not scale or even work for enterprise environments. Please reconsider looking into this. |
Add the version package from `prometheus/common` to allow breaking the circular dependency between `prometheus/client_golang` and `prometheus/common`. Related to: prometheus/common#58 Signed-off-by: SuperQ <superq@gmail.com>
Add the version package from `prometheus/common` to allow breaking the circular dependency between `prometheus/client_golang` and `prometheus/common`. * Remove support for Go older than 1.18. Related to: prometheus/common#58 Signed-off-by: SuperQ <superq@gmail.com>
|
Relevant discussion is happening here: https://cloud-native.slack.com/archives/C01AUBA4PFE/p1704099677796159 |
Migrate the `versoin` package from `github.com/prometheus/common` to `github.com/prometheus/client_golang` in order to break the ciricular dependency. * Make `version` a top level package because it uses `init()` to populate data. Related to: prometheus/common#58 Signed-off-by: SuperQ <superq@gmail.com>
Migrate the `version` package from `github.com/prometheus/common` to `github.com/prometheus/client_golang` in order to break the ciricular dependency. * Make `version` a top level package because it uses `init()` to populate data. Related to: prometheus/common#58 Signed-off-by: SuperQ <superq@gmail.com>
Migrate the `version` package from `github.com/prometheus/common` to `github.com/prometheus/client_golang` in order to break the circular dependency. * Make `version` a top level package because it uses `init()` to populate data. Related to: prometheus/common#58 Signed-off-by: SuperQ <superq@gmail.com>
Migrate the `version` package from `github.com/prometheus/common` to `github.com/prometheus/client_golang` in order to break the circular dependency. * Make `version` a top level package because it uses `init()` to populate data. Related to: prometheus/common#58 Signed-off-by: SuperQ <superq@gmail.com>
|
Circular dependency has been solved after moving the version collector to client_golang! |
Hey,
I just got reported (I had not noticed), that there is a circular dependency between common and client_golang, which is pretty problematic for transitions, backports, etc.
In the file
version/info.go, the client library is imported to create metrics with version information. Is there any sane way to break this cycle? Otherwise, I guess I should just merge the packages; they can't be used independently anyway.The text was updated successfully, but these errors were encountered: