ENT-13808: Improve release history generation and download handling#51
ENT-13808: Improve release history generation and download handling#51sarakthon wants to merge 2 commits intocfengine:mainfrom
Conversation
…to top of generate_release_information_impl
|
Thanks for submitting a PR! Maybe @larsewi can review this? |
| generate_release_history() | ||
|
|
||
| generate_git_tags_map() | ||
|
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
You did the removal part in the wrong commit (2nd commit).
| 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) |
There was a problem hiding this comment.
These formatting changes should not be in the same commit.
| def process_release_type(folder, download_func): | ||
| # Function for processing either community or enterprise releases |
There was a problem hiding this comment.
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?
| # Downloading releases.json: | ||
| try: | ||
| releases_data = get_json(ENTERPRISE_RELEASES_URL) | ||
| return requests.get(ENTERPRISE_RELEASES_URL) |
There was a problem hiding this comment.
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.
| 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}.") |
There was a problem hiding this comment.
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 data3 participants
No description provided.