Skip to content

Accept-Encoding: gzip#86

Open
shamilbi wants to merge 5 commits intoido50:mainfrom
shamilbi:gzip
Open

Accept-Encoding: gzip#86
shamilbi wants to merge 5 commits intoido50:mainfrom
shamilbi:gzip

Conversation

@shamilbi
Copy link
Copy Markdown
Collaborator

@shamilbi shamilbi commented Mar 2, 2026

use "Accept-Encoding": "gzip" to reduce traffic

Comment thread morgan/__init__.py Outdated
Comment thread morgan/__init__.py Outdated
Comment thread morgan/__init__.py Outdated
Comment thread morgan/__init__.py Outdated
@shamilbi shamilbi requested a review from grische March 3, 2026 18:18
Copy link
Copy Markdown
Contributor

@grische grische left a comment

Choose a reason for hiding this comment

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

Also, please clean up your commit history.

As we are having a lot of back and forth here, you can also ask some modern LLM for a review of your changes first and address those before updating the PR.

Claude Free tier (from claude.ai) should already be sufficient enough to give you some good feedback.

Comment thread morgan/__init__.py
Comment on lines +582 to +583
# ruff: noqa: S310
with urllib.request.urlopen(fileinfo["url"]) as inp, open(target, "wb") as out:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
# ruff: noqa: S310
with urllib.request.urlopen(fileinfo["url"]) as inp, open(target, "wb") as out:
with urllib.request.urlopen(fileinfo["url"]) as inp, open(target, "wb") as out: # ruff: noqa: S310

Otherwise the noqa applies to the rest of the file.

Comment thread morgan/__init__.py
# At least one of the tags must match ALL of our environments
for tag in fileinfo["tags"]:
(intrp_name, intrp_ver) = parse_interpreter(tag.interpreter)
intrp_name, intrp_ver = parse_interpreter(tag.interpreter)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This change is unrelated to the PR and should be not part of the PR or at least in a separate commit explaining why

Comment thread morgan/utils.py
super().__setitem__(key, value)


def download_req(index_url: str, req_name: str) -> tuple[dict, str]:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The name of the function is oddly chosen. Either change the function to be a generic function that can be reused (hence has a reason to be in utils.py) or move it into the place where it is meant to be and name it more specifically.

Comment thread morgan/utils.py
self[key].extend(value)
else:
super().__setitem__(key, value)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This function is OK to have, but right now it is doing code duplication within metadata.py.
Either use the function everywhere, or not create a function at all.

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.

2 participants