Skip to content

ENT-13808: Improve release history generation and download handling #51

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.

Sign up for GitHub

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

Open
sarakthon wants to merge 2 commits into
base: main
Choose a base branch
from
+33 −24
Open

ENT-13808: Improve release history generation and download handling #51

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f43d0c7
ENT-13808: Move generate_release_history() and generate_git_tags_map …
sarakthon Apr 9, 2026
2c27558
ENT-13808: Download releases.json directly to file and parse afterwards
sarakthon Apr 9, 2026
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
57 changes: 33 additions & 24 deletions src/cfengine_cli/masterfiles/generate_release_information.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
import requests
from cfengine_cli.masterfiles.download import (
ENTERPRISE_RELEASES_URL,
COMMUNITY_RELEASES_URL,
Expand Down Expand Up @@ -29,6 +30,10 @@ def generate_release_information_impl(
omit_download=False, check=False, min_version=None
):
if not omit_download:
generate_release_history()

generate_git_tags_map()

Comment on lines +32 to +35
Copy link
Copy Markdown
Member

@olehermanse olehermanse Apr 10, 2026

Choose a reason for hiding this comment

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

You also need to remove it from the end, now it runs twice :)

Also, the commit message should mention why you're doing this. That's almost always more important than restating what you did unless it's really obvious.

Copy link
Copy Markdown
Member

@olehermanse olehermanse Apr 10, 2026

Choose a reason for hiding this comment

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

You did the removal part in the wrong commit (2nd commit).

print("Downloading masterfiles...")

downloaded_versions = download_all_versions(DOWNLOAD_PATH, min_version)
Expand All @@ -49,10 +54,6 @@ def generate_release_information_impl(
)
generate_vcf_download(DOWNLOAD_PATH, downloaded_versions)

generate_release_history()

generate_git_tags_map()

if check:
print(
"Downloading releases of masterfiles from git (github.com) and generating "
Expand All @@ -77,43 +78,37 @@ def generate_release_information_impl(
def download_enterprise_releasedata():
# Downloading releases.json:
try:
releases_data = get_json(ENTERPRISE_RELEASES_URL)
return requests.get(ENTERPRISE_RELEASES_URL)
Copy link
Copy Markdown
Member

@olehermanse olehermanse Apr 10, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Here you change the return type from json / dict to a requests response object. However, where you use this, you only use response.content, right? Maybe it's cleaner to just return .content? When I write functions that essentially wrap requests in various ways, I usually like to not "leak" the fact that it is requests.


except CFBSNetworkError:
except requests.RequestException:
raise CFBSExitError(
"Downloading CFEngine release data failed - check your Wi-Fi / network settings."
)

return releases_data


def download_community_releasedata():
# Downloading community/releases.json
try:
releases_data = get_json(COMMUNITY_RELEASES_URL)

except CFBSNetworkError:
return requests.get(COMMUNITY_RELEASES_URL)
except requests.RequestException:
raise CFBSExitError(
"Downloading CFEngine release data failed - check your Wi-Fi / network settings."
)

return releases_data


def process_release_type(folder, download_func):
# Function for processing either community or enterprise releases
Comment on lines 99 to 100
Copy link
Copy Markdown
Member

@olehermanse olehermanse Apr 10, 2026

Choose a reason for hiding this comment

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

This function was here already, but looking at it, I wonder if it unnecessarily complex.

Why is it necessary to pass in a download function, instead of just a download URL?

release_data = download_func()

write_json_pretty(f"./{folder}/releases.json", release_data)
releases_path = f"./{folder}/releases.json"
print(f"\nDownloading release data for {folder}...")
response = download_func()
# Save raw file to preserve exact format from website
save_to_file(releases_path, response.content)

release_data = load_json_from_file(releases_path)
stable_releases = get_stable_releases(release_data)

file_checksums_dict = build_release_history(stable_releases)

write_version_files(stable_releases, folder)

sorted_releases = sort_release_data(file_checksums_dict)

write_json(f"./{folder}/checksums.json", sorted_releases)
Comment on lines 108 to 112
Copy link
Copy Markdown
Member

@olehermanse olehermanse Apr 10, 2026

Choose a reason for hiding this comment

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

These formatting changes should not be in the same commit.



Expand Down Expand Up @@ -188,7 +183,21 @@ def write_version_files(stable_releases, folder):
write_json(f"./{folder}/versions/{version}.json", version_data)


def write_json_pretty(path, data):
# Writes release information in same format as on cfengine.com
with open(path, "w", encoding="utf-8") as f:
json.dump(data, f, indent=2, ensure_ascii=False)
def save_to_file(path, content):
try:
with open(path, "wb") as f:
f.write(content)
print(f"Saved {path}")

except OSError:
raise CFBSExitError(f"Failed to write file {path}.")


def load_json_from_file(path):
# Open saved file and return as dict
try:
with open(path, "r", encoding="utf-8") as f:
return json.load(f)

except OSError:
raise CFBSExitError(f"Failed to read file {path}.")
Comment on lines +186 to +203
Copy link
Copy Markdown
Member

@olehermanse olehermanse Apr 10, 2026

Choose a reason for hiding this comment

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

I was thinking we could make a function which does "everything" you need, without saving the data to file and then reading it from file again;

def json_get_save_return(url: str, path: str) -> dict:
    r = requests.get(url)
    content = r.content
    with open(path, "wb") as f:
        f.write(content)
    # TODO: Add some error handling
    data = json.loads(content)
    return data

Loading