Closed avara1986 closed 3 years ago
@Agalin @alexppg is it ok as a simple service discovery for Consul?
I want to add these days ACL validations and tokens :+1: https://www.consul.io/api-docs/acl/tokens
Changes Missing Coverage | Covered Lines | Changed/Added Lines | % | ||
---|---|---|---|---|---|
pyms/cmd/main.py | 16 | 17 | 94.12% | ||
pyms/flask/app/utils.py | 6 | 7 | 85.71% | ||
pyms/flask/services/service_discovery.py | 39 | 40 | 97.5% | ||
pyms/logger/logger.py | 8 | 9 | 88.89% | ||
tests/test_metrics.py | 9 | 10 | 90.0% | ||
<!-- | Total: | 307 | 312 | 98.4% | --> |
Files with Coverage Reduction | New Missed Lines | % | ||
---|---|---|---|---|
pyms/crypt/driver.py | 1 | 96.97% | ||
<!-- | Total: | 1 | --> |
Totals | |
---|---|
Change from base Build 1088: | -0.01% |
Covered Lines: | 1976 |
Relevant Lines: | 1996 |
ACL is nice but I wonder how would client-specific configuration options be handled? Is there any plan (maybe in tracer as there are 2 clients already) how should that be done?
I added all the requests you did @Agalin :)
consulate
. The Version 0.6.0 isn't work correctly, and not exists new versions since 2015 (https://github.com/gmr/consulate/issues/120)ms.service_discovery.client.agent.checks()
(see examples)Woah. There is a lot of linter fixes.
Service itself looks fine to me although I've been talking about forking (e.g. into python-microservices
org) not vendoring consulate
. Vendored is not useful to the community outside of pyms (and for this reason won't get much of external support), harder to provide fixes for (need whole pyms test suite to pass) and increases bundle size for people completely uninterested in service discovery (although this is a minor issue as it doesn't seem to have many dependencies).
I moved all code into PyMS because I can't upload a new version of consulte to Pip. But... I could create a fork and create a package "pyms-consulate", upload to pip and use it as dependency, in your opinion @Agalin , is it a good solution?
That's exactly what I mean by fork
in this context. 😄 Just so development on consulate can be continued somewhere.
+1 to forking and have a new package!
Ready this PR! :)
This is the fork: https://github.com/python-microservices/consulate And the package with last changes of consulate: https://pypi.org/project/py-ms-consulate/
do you see anything else @alexppg @Agalin ? is it ready to merge? :)
Yeah imo it's fine.
Fixes #190 #178
Changes proposed in this PR: