refactor: migrate to viper configure manage #1946
Conversation
|
✔️ Deploy Preview for apisix-dashboard ready! 🔨 Explore the source changes: 92eeef1 🔍 Inspect the deploy log: https://app.netlify.com/sites/apisix-dashboard/deploys/60e67a5136b608000732921d 😎 Browse the preview: https://deploy-preview-1946--apisix-dashboard.netlify.app |
Codecov Report
@@ Coverage Diff @@
## master #1946 +/- ##
==========================================
+ Coverage 66.79% 69.13% +2.33%
==========================================
Files 182 60 -122
Lines 6575 3428 -3147
Branches 753 0 -753
==========================================
- Hits 4392 2370 -2022
+ Misses 1907 785 -1122
+ Partials 276 273 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
bzp2010
force-pushed
the
feat-migrate-viper
branch
9 times, most recently
from
4403a17 to
31526f1
Compare
bzp2010
changed the title
2021-06-22 Update1There are some key problems in refactoring.
I'm trying to refactor the server module and the service daemon module. |
bzp2010
force-pushed
the
feat-migrate-viper
branch
from
31526f1 to
a3a5071
Compare
bzp2010
force-pushed
the
feat-migrate-viper
branch
2 times, most recently
from
120a4cb to
92eeef1
Compare
| github.com/prometheus/client_golang v1.8.0 // indirect | ||
| github.com/satori/go.uuid v1.2.0 | ||
| github.com/shiningrush/droplet v0.2.6-0.20210127040147-53817015cd1b | ||
| github.com/shiningrush/droplet/wrapper/gin v0.2.1 | ||
| github.com/sirupsen/logrus v1.7.0 // indirect | ||
| github.com/sony/sonyflake v1.0.0 | ||
| github.com/spf13/cobra v0.0.3 | ||
| github.com/spf13/pflag v1.0.5 // indirect | ||
| github.com/spf13/viper v1.8.1 |
There was a problem hiding this comment.
I remember that the license of a dependency of viper was not compatible with Apache 2.0, please confirm.
There was a problem hiding this comment.
Hi, @nic-chen. I'm sorry for the slow response.
I used google/go-licenses to generate a list of the spf13/viper's dependencies and licenses.
Include Apache-2.0, MIT, MPL-2.0, BSD-2-Clause and BSD-3-Clause.
And these licenses are also used in current Manager API dependencies.
There was a problem hiding this comment.
I'm not sure about MPL-2.0, @moonming please have a look, thanks.
There was a problem hiding this comment.
Description
Viper library uses the API of hashicorp/hcl open source repository using MPL-2.0 license, but does not use its code directly.
Solution
If importing a library using MPL-2.0 license is not feasible, I will consider implementing a shim code and replacing it with go mod replace. It emulates the API of that library and provides empty results.
It's like the following image.
So is such a solution feasible?
There was a problem hiding this comment.
I searched the go project under apache, and many projects also use viper
for example:
https://github.com/apache/skywalking-banyandb
https://github.com/apache/camel-k
I have no problem here.
There was a problem hiding this comment.
MPL-2.0 maybe can NOT in apache projects: https://www.apache.org/legal/resolved.html
There was a problem hiding this comment.
Hi, @moonming. Sorry to bother you again.
-
I have read this specification https://www.apache.org/legal/resolved.html and made a query in JIRA, I found the following discussion, LEGAL-562: Handling dependency licenses for the Solr Operator, a go project.
I think our situation is similar to that in the article. We do not include the code under the MPL-2.0 license in the code distribution. The code is downloaded by the user during the compilation process. In the docker image involving binary distribution (It belongs to class B in the specification, and we can distribute it using binary), we should modify theLICENSEandNOTICEfiles and include them in the image as a reminder. Just like these project, apache/skywalking-banyandb.
usage eg. in apache/skywalking-banyandb
https://github.com/apache/skywalking-banyandb/blob/c6314fe15b56143836e8969bd756a1310779b3b0/go.mod#L20 -
If we really can't include the dependencies of this library, can the above scheme of replacing the dependent library code with shim? here
🤔 What do you think of this? Thx.
There was a problem hiding this comment.
ping @moonming Any changes here?
There was a problem hiding this comment.
Having a dependency with a Category B license is fine
Please answer these questions before submitting a pull request, or your PR will get closed.
Why submit this pull request?
What changes will this PR take into?
The configuration management is implemented by
viperlibrary instead of reading yaml file directly.It will provide support for YAML, JSON, TOML and other configuration file formats, and even remote loading through etcd, which will be controlled by
-cor--configparameters.And no incompatible updates introduced.
Related issues
part of #1931
Checklist: