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

🎨 improve release notes (ignore minor version) #6393

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,14 @@ async def create_and_cache_statics_json(app: web.Application) -> None:
and product.vendor
and (template_url := product.vendor.get("release_notes_url_template", None))
):
# template URL should be somethign like:
# As pattern vX.Y.Z is already guaranteed, we replace the the minor version with 0
parts = vtag.split(".")
parts[-1] = "0"
updated_vtag = ".".join(parts)

# template URL should be something like:
# https://github.com/ITISFoundation/osparc-issues/blob/master/release-notes/osparc/{vtag}.md
data["vcsReleaseUrl"] = template_url.format(vtag=vtag)
data["vcsReleaseUrl"] = template_url.format(vtag=updated_vtag)
matusdrobuliak66 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

This solution is possible but IMO very restrictive. The main idea here should rather be passing as much information to the template_url formatter so that we are as flexible as possible to compose the url
Let me illustrate you with an example, if we instead deduce and pass more granulated information to the formatter like major, minor, etc

#lets assume
template_url = product.vendor["release_notes_url_template"]

data["vcsReleaseUrl"] = template_url.format(vtag=vtag, major=major, minor=minor, patch=patch)

then in the database (which s more convenient) we could redo at any time the url template without. Here some examples

release_notes_url_template = "https://mydoc/{vtag}".  # old template format
release_notes_url_template = "https://mydoc/v{major}.{minor}.0" # new 
# ...
release_notes_url_template = "https://mydoc/v{major}/{minor}.md" # my prediction for 2025's format

So, in summary

  1. do not modify vtag but rather inject more information to the formatter. Note that your solution, even if correct, is "removing" information.
  2. if _RE_PRODUCTION_RELEASE_VERSION is so relevant, the it MUST be part of the definition of app_settings.SIMCORE_VCS_RELEASE_TAG, i.e. we should not accept non-conformal strings for this input. If you do not want to fail upon validation then create a pre-validator that nullifies with a warning it if it is invalid
    • if the format of this field is guranteed, then it is very easy to extract major, minor and patch from this. e.g. v = packaging.version.Version(app_settings.SIMCORE_VCS_RELEASE_TAG.removeprefix("v")), v.major, v.minor


data_json = json_dumps(data)
_logger.debug("Front-end statics.json: %s", data_json)
Expand Down
6 changes: 6 additions & 0 deletions services/web/server/tests/unit/with_dbs/01/test_statics.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,12 @@ def mock_product_vendor(postgres_db: sa.engine.Engine, template_url: str) -> Non
"https://example.com/no_vtag.md",
id="vtag_not_repalced_if_missing",
),
pytest.param(
"v1.75.4",
"https://example.com/releases/some_target_{vtag}.md",
"https://example.com/releases/some_target_v1.75.0.md",
matusdrobuliak66 marked this conversation as resolved.
Show resolved Hide resolved
id="production_replacement_hotfix_version",
),
],
)
async def test_create_and_cache_statics_json_vendor_vcs_overwrite(
Expand Down
Loading