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

Keep a consistant order for hashes for get_package_hashes. #131

Merged
merged 4 commits into from
Jan 7, 2022

Conversation

pcorpet
Copy link
Contributor

@pcorpet pcorpet commented Jan 3, 2022

Fixes #126

@MarkusH
Copy link
Contributor

MarkusH commented Jan 3, 2022

This change raises an error for me on Python 3.9 and 3.10:

$ python -m hashin -r requirements.txt django
Traceback (most recent call last):
  File "/usr/lib/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/home/markus/Coding/hashin/hashin.py", line 855, in <module>
    sys.exit(main())
  File "/home/markus/Coding/hashin/hashin.py", line 837, in main
    return run(
  File "/home/markus/Coding/hashin/hashin.py", line 135, in run
    return run_packages(specs, requirements_file, *args, **kwargs)
  File "/home/markus/Coding/hashin/hashin.py", line 249, in run_packages
    for i, release in enumerate(data["hashes"], key=lambda r: r["hash"]):
TypeError: 'key' is an invalid keyword argument for enumerate()
$ hashin -r requirements.txt django
Traceback (most recent call last):
  File "/home/markus/Coding/hashin/venv39/bin/hashin", line 33, in <module>
    sys.exit(load_entry_point('hashin', 'console_scripts', 'hashin')())
  File "/home/markus/Coding/hashin/hashin.py", line 837, in main
    return run(
  File "/home/markus/Coding/hashin/hashin.py", line 135, in run
    return run_packages(specs, requirements_file, *args, **kwargs)
  File "/home/markus/Coding/hashin/hashin.py", line 249, in run_packages
    for i, release in enumerate(data["hashes"], key=lambda r: r["hash"]):
TypeError: 'key' is an invalid keyword argument for enumerate()

@pcorpet
Copy link
Contributor Author

pcorpet commented Jan 4, 2022

Oops, I was sloppy, sorry about that.

@MarkusH
Copy link
Contributor

MarkusH commented Jan 4, 2022

Works for me. Thank you!

@peterbe
Copy link
Owner

peterbe commented Jan 5, 2022

@pcorpet
Would you mind editing the matrix in .github/workflows/python.yml and the Python version lists in tox.ini as part of this PR?

@peterbe
Copy link
Owner

peterbe commented Jan 5, 2022

In fact, perhaps it would be best to move the other linting tasks that, currently, 3.8 does into 3.10 in the tox file.

Copy link
Owner

@peterbe peterbe left a comment

Choose a reason for hiding this comment

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

Great work!!
But because of the comments from @MarkusH that it, did behave differently in 3.9 and 3.10 I would not feel confident merging this till we have 3.9 and 3.10 in the test matrix.

As mentioned, I'm cool with you adding that into this PR if it's a bit impure. Or, if you want to put up a quick PR to just add 3.9 and 3.10 to the tests first, I'm happy to merge that quickly.

@pcorpet
Copy link
Contributor Author

pcorpet commented Jan 5, 2022

@peterbe it was actually failing on all versions before my fixes. But I'm all for clean commits and PRs so I'll send you another PR.

@pcorpet
Copy link
Contributor Author

pcorpet commented Jan 5, 2022

I also realize I forgot to update the version history...

Copy link
Contributor

@MarkusH MarkusH left a comment

Choose a reason for hiding this comment

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

Nice!

@pcorpet
Copy link
Contributor Author

pcorpet commented Jan 7, 2022

I've rebased so that CI runs on python 3.9 and 3.10: ready to merge.

@peterbe peterbe merged commit bca468b into peterbe:master Jan 7, 2022
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.

Order of hashes from get_package_hashes
3 participants