better versioning#546
Conversation
masci
left a comment
masci
left a comment
There was a problem hiding this comment.
A couple of nits, overall I think we should add more comments/docs about how the version is composed
| const AgentVersion = "6.0.0+Χελωνη" | ||
| var AgentVersion string | ||
|
|
||
| var agentVersionDefault = "6.0.0" |
There was a problem hiding this comment.
Can we change 6.0.0 to something that works in case the agent is custom built? Something like 0.0.0
There was a problem hiding this comment.
There are a lot of places in the agent where it is assumed to be 6.0.0. If you look in the tests, you'll see this:
--- FAIL: TestGetMultipleEndpoints (0.00s)
Error Trace: config_test.go:59
Error: Not equal: map[string][]string{"https://foo.datadoghq.com":[]string{"someapikey"}, "https://6-0-0-app.agent.datadoghq.com":[]string{"fakeapikey", "fakeapikey2", "fakeapikey3"}} (expected)
!= map[string][]string{"https://0-0-0-app.agent.datadoghq.com":[]string{"fakeapikey", "fakeapikey2", "fakeapikey3"}, "https://foo.datadoghq.com":[]string{"someapikey"}} (actual)
Are you sure you want that as the default?
There was a problem hiding this comment.
We shouldn't hardcode the version number, hopefully this agent won't be 6 for a long time. I can address this in a separate PR though.
|
|
||
| func init() { | ||
| if AgentVersion == "" { | ||
| AgentVersion = "6.0.0" |
There was a problem hiding this comment.
this should be = agentVersionDefault
| described_version = check_output(["git", "describe", "--tags"], stderr=subprocess.STDOUT).strip() | ||
| except: | ||
| described_version = "" | ||
| version_match = re.findall(r"^v?(\d+\.\d+\.\d+)", described_version) |
There was a problem hiding this comment.
can you add an example string the regex would match as a comment?
| else: | ||
| version = "6.0.0" | ||
|
|
||
| commits_since_version_match = re.findall(r"^.*-(\d+)\-g[0-9a-f]+$", described_version) |
There was a problem hiding this comment.
what are commits_since_version_match? Can you add a comment and an example string we'd match?
|
|
||
| pre_regex = "" | ||
| if commits_since_version == 0: | ||
| pre_regex = r"^v?\d+\.\d+\.\d+(?:-|\.)([0-9A-Za-z.-]+)$" |
|
@masci I made the changes you asked for! |
3 participants
What does this PR do?
This is not a very complex change, it takes some of the logic we use in omnibus, translates it to python, and then sets the version
Motivation
We needed a more meaningful version for the agent
Additional Notes
Anything else we should know when reviewing?