Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add version metadata #4874

Merged
merged 68 commits into from
Dec 13, 2019
Merged

Add version metadata #4874

merged 68 commits into from
Dec 13, 2019

Conversation

hithwen
Copy link
Contributor

@hithwen hithwen commented Oct 24, 2019

Adds inventories to PG, also stop using negative versions for betas
Depends on #4844

@codecov
Copy link

codecov bot commented Oct 24, 2019

@hithwen hithwen marked this pull request as ready for review October 31, 2019 13:33
@hithwen hithwen requested a review from a team as a code owner October 31, 2019 13:33
AlexandreYang
AlexandreYang previously approved these changes Oct 31, 2019
postgres/datadog_checks/postgres/version_utils.py Outdated Show resolved Hide resolved
postgres/datadog_checks/postgres/version_utils.py Outdated Show resolved Hide resolved
@therve therve changed the title PG inventories Submit version metadata Nov 5, 2019
@AlexandreYang AlexandreYang changed the title Submit version metadata Add version metadata Nov 5, 2019
Copy link
Contributor

@ofek ofek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

postgres/datadog_checks/postgres/version_utils.py Outdated Show resolved Hide resolved
@hithwen hithwen merged commit 0e70cca into master Dec 13, 2019
@hithwen hithwen deleted the julia/pg-inventories branch December 13, 2019 09:26
'version.patch': '4',
'version.scheme': 'semver',
}
assert expected == version
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nit) This could be a parameterized test.

Currently if the firs case fail, the following cases are not run.

db.cursor().fetchone.return_value = ['11nightly3']
assert get_version(db) == VersionInfo(11, -1, 3)
version = parse_version('11nightly3')
assert version == VersionInfo(11, 0, 0, 'nightly.3')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nit) This could be a parameterized test.

Currently if the firs case fail, the following cases are not run.

@@ -749,3 +751,4 @@ def check(self, instance):
self.db.commit()
except Exception as e:
self.log.warning("Unable to commit: %s", e)
self._version = None # We don't want to cache versions between runs to capture minor updates for metadata
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nit) In this case, maybe we should just pass down the version as method parameter instead of caching it on self._version. Seems easier to understand and less error prone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants