Skip to content

MDB-40234: added version to logs and metrics#105

Closed
aak74 wants to merge 6 commits intoyandex:mainfrom
aak74:MDB-40234-version
Closed

MDB-40234: added version to logs and metrics#105
aak74 wants to merge 6 commits intoyandex:mainfrom
aak74:MDB-40234-version

Conversation

@aak74
Copy link
Copy Markdown
Collaborator

@aak74 aak74 commented Nov 7, 2025

No description provided.

@aak74 aak74 requested a review from a team as a code owner November 7, 2025 15:20
Comment thread src/__init__.py Outdated
try:
# Try to find package.release in the installation directory
version_file = '/opt/yandex/pgconsul/package.release'
if os.path.exists(version_file):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No need to check if file exists if you already have try..except

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

Comment thread src/cli.py Outdated

db_state = _get_db_state(conf)
return {**db_state, **zk_state}
return {'version': __version__, **db_state, **zk_state}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

And... it might be incorrect!
If you have updated package on a system, but didn't restart daemon, it will report newer version, while old one would be maintaining HA of a cluster.

I'd suggest to write daemon version into state file, and read it in cli utility.
And maybe even report versions of cli and daemon separately.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

Comment thread src/version.py Outdated
Version management for pgconsul.
"""

STATE_FILE = '/tmp/pgconsul.state'
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why do we need to introduce another file, if we already have status file: https://github.com/yandex/pgconsul/blob/main/src/helpers.py#L242?
Let's just add a field to its json contents?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

fixed

Comment thread Makefile
| xargs sed -i -e 's|$(INSTALL_DIR)|/opt/yandex/pgconsul|' \
|| true
# Write version to package.release if VERSION is not "-"
test "$(VERSION)" != "-" && echo $(VERSION) > $(DESTDIR)/opt/yandex/pgconsul/package.release || true
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
test "$(VERSION)" != "-" && echo $(VERSION) > $(DESTDIR)/opt/yandex/pgconsul/package.release || true
test "$(VERSION)" != "-" && echo $(VERSION) > $(INSTALL_DIR)/package.release || true

Comment thread src/__init__.py
"""Initialize version: read from package.release file and return it"""
try:
# Try to find package.release in the installation directory
with open('/opt/yandex/pgconsul/package.release', 'r') as f:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It should not be a hardcoded final path here in Python code.
Final install path is defined in Makefile, and can be set to some other path, which Python module should adapt to.

I'd suggest to use a common Python method https://docs.python.org/3/library/importlib.resources.html to access this resource file, and install it to corresponding location.

@aak74 aak74 closed this Jan 29, 2026
@aak74
Copy link
Copy Markdown
Collaborator Author

aak74 commented Jan 29, 2026

Implemented outside the service

@aak74 aak74 reopened this Jan 30, 2026
@mialinx mialinx closed this Apr 2, 2026
@mialinx
Copy link
Copy Markdown
Collaborator

mialinx commented Apr 2, 2026

we decided to simplify and rewrite the code

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants