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.

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

Jump to bottom

Remove use of distutils #120

Merged
merged 3 commits into from
Nov 14, 2023
Merged

Remove use of distutils #120

merged 3 commits into from
Nov 14, 2023

Conversation

edgarrmondragon
Copy link
Contributor

@edgarrmondragon edgarrmondragon commented Oct 14, 2023 •
edited
Loading

@edgarrmondragon
Copy link
Contributor Author

@microsoft-github-policy-service agree

@edgarrmondragon edgarrmondragon marked this pull request as ready for review October 14, 2023 02:58
edgarrmondragon
edgarrmondragon commented Oct 14, 2023
View reviewed changes
msal_extensions/cache_lock.py
@@ -21,7 +21,7 @@ def __init__(self, lockfile_path):
self._lockpath = lockfile_path
# Support for passing through arguments to the open syscall was added in v1.4.0
open_kwargs = ({'buffering': 0}
if LooseVersion(portalocker.__version__) >= LooseVersion("1.4.0") else {})
if Version(portalocker.__version__) >= Version("1.4.0") else {})
Copy link
Contributor Author

@edgarrmondragon edgarrmondragon Oct 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think LooseVersion is really needed here and Version should be good enough.

Copy link
Contributor

Choose a reason for hiding this comment

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

The LooseVersion() was introduced by @bluca . What do you think with this change, Luca?

Copy link
Contributor

Choose a reason for hiding this comment

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

looks good to me, thanks

@rayluo rayluo added the enhancement New feature or request label Oct 14, 2023
@edgarrmondragon
Copy link
Contributor Author

Thanks @bluca! Tests are passing for me locally, even on 2.7:

Test results on 2.7 
$ pytest -vvv
============================================================================================== test session starts ===============================================================================================
platform darwin -- Python 2.7.18, pytest-4.6.11, py-1.11.0, pluggy-0.13.1 -- /oss/microsoft-authentication-extensions-for-python/venv2.7/bin/python
cachedir: .pytest_cache
rootdir: /oss/contrib/microsoft-authentication-extensions-for-python
collected 18 items / 1 skipped / 17 selected                                                                                                                                                                     

tests/test_agnostic_backend.py::test_file_token_cache_roundtrip SKIPPED                                                                                                                                    [  5%]
tests/test_agnostic_backend.py::test_current_platform_cache_roundtrip_with_persistence_builder SKIPPED                                                                                                     [ 11%]
tests/test_agnostic_backend.py::test_persisted_token_cache SKIPPED                                                                                                                                         [ 16%]
tests/test_agnostic_backend.py::test_file_not_found_error_is_not_raised PASSED                                                                                                                             [ 22%]
tests/test_cache_lock_file_perf.py::test_lock_for_normal_workload PASSED                                                                                                                                   [ 27%]
tests/test_cache_lock_file_perf.py::test_lock_for_high_workload PASSED                                                                                                                                     [ 33%]
tests/test_cache_lock_file_perf.py::test_lock_for_timeout PASSED                                                                                                                                           [ 38%]
tests/test_crossplatlock.py::test_ensure_file_deleted PASSED                                                                                                                                               [ 44%]
tests/test_macos_backend.py::test_keychain_roundtrip PASSED                                                                                                                                                [ 50%]
tests/test_macos_backend.py::test_osx_token_cache_roundtrip SKIPPED                                                                                                                                        [ 55%]
tests/test_persistence.py::test_file_persistence PASSED                                                                                                                                                    [ 61%]
tests/test_persistence.py::test_nonexistent_file_persistence PASSED                                                                                                                                        [ 66%]
tests/test_persistence.py::test_file_persistence_with_data_protection SKIPPED                                                                                                                              [ 72%]
tests/test_persistence.py::test_nonexistent_file_persistence_with_data_protection SKIPPED                                                                                                                  [ 77%]
tests/test_persistence.py::test_keychain_persistence PASSED                                                                                                                                                [ 83%]
tests/test_persistence.py::test_nonexistent_keychain_persistence PASSED                                                                                                                                    [ 88%]
tests/test_persistence.py::test_libsecret_persistence SKIPPED                                                                                                                                              [ 94%]
tests/test_persistence.py::test_nonexistent_libsecret_persistence SKIPPED                                                                                                                                  [100%]

===================================================================================== 10 passed, 9 skipped in 11.51 seconds ======================================================================================

So I pushed another commit to use fail-fast: false and let the non-2.7 tests run.

@mickare
Copy link

mickare commented Oct 26, 2023 •
edited
Loading

Hey @rayluo & @bluca , how are you doing?

Could you priotize this to be merged soon? It's a blocker for anyone using the azure-identity library with Python3.12.

Thanks! 🙏

@mickare mickare mentioned this pull request Oct 31, 2023
Closed
@rayluo rayluo merged commit d7f7623 into AzureAD:dev Nov 14, 2023
@edgarrmondragon edgarrmondragon deleted the remove-distutils branch November 14, 2023 15:29
@edgarrmondragon
Copy link
Contributor Author

Hey folks 👋

Is it worth making a new release with this change?

@rayluo rayluo mentioned this pull request Dec 9, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bluca bluca bluca approved these changes

@rayluo rayluo rayluo approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

distutils was removed in Python 3.12 getting "distutils Version classes are deprecated" warnings
4 participants
@edgarrmondragon @mickare @bluca @rayluo