Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions src/dockerflow/version.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,22 @@

def get_version(root):
"""
Load and return the contents of version.json.
Load and return the contents of version.json. If a DOCKERFLOW_VERSION
env var is set, sets the "version" attribute to the env var value.
:param root: The root path that the ``version.json`` file will be opened
:type root: str
:returns: Content of ``version.json`` or None
:rtype: dict or None
"""
version_json = os.path.join(root, "version.json")
version_env_var = os.getenv("DOCKERFLOW_VERSION")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why add this additional source of information instead of using the version.json file?

Copy link
Contributor Author

@alexcottner alexcottner Jul 11, 2025

Choose a reason for hiding this comment

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

We adjusted the remote-settings release process to re-tag the existing image instead of building a fresh one (link). This reduces our release times and allows us to use the exact same image that's been working great already in dev/stage.

But, the version file doesn't then get rebuilt so our version endpoint is incorrect. My best ideas to solve this were:

  1. Allow for an optional version parameter to be sent (this PR basically)
  2. Update and mount a configmap for the version.json file on deploy

This felt a little easier to me but I'm open to option 2.

Copy link
Collaborator

@willkg willkg Jul 11, 2025

Choose a reason for hiding this comment

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

I think sliding the version in outside of the version.json file doesn't make sense and breaks the contract of version.json.

I think it'd be better to have a short Dockerfile template, expand the template with the image details of the stage image you're using, and then COPY the updated version.json file.

You should talk with @grahamalama and see if that's something we can put in deploy-actions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤔 Yeah that's another option and is pretty simple to implement

if os.path.exists(version_json):
with open(version_json, "r") as version_json_file:
return json.load(version_json_file)
version_info = json.load(version_json_file)
if version_env_var != None:
version_info["version"] = version_env_var
return version_info
if version_env_var != None:
return { "version": version_env_var }
return None
20 changes: 20 additions & 0 deletions tests/core/test_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, you can obtain one at http://mozilla.org/MPL/2.0/.
import json
import os

from dockerflow.version import get_version

Expand All @@ -18,3 +19,22 @@ def test_get_version(tmpdir):
def test_no_version_json(tmpdir):
version = get_version(str(tmpdir))
assert version is None

def test_env_var_override(tmpdir, mocker):
content = {"spam": "eggs"}
mocker.patch.dict(os.environ, { "DOCKERFLOW_VERSION": "foo"})
version_json = tmpdir.join("version.json")
version_json.write(json.dumps(content))

version = get_version(str(tmpdir))
assert version == {
"spam": "eggs",
"version": "foo"
}

def test_env_var_override_with_no_json(tmpdir, mocker):
mocker.patch.dict(os.environ, { "DOCKERFLOW_VERSION": "foo"})
version = get_version(str(tmpdir))
assert version == {
"version": "foo"
}
Loading