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

feat: replace lib imports by importlib #192

Merged
merged 7 commits into from
Aug 8, 2023

Conversation

Andromelus
Copy link
Contributor

Goal: fix #191
I am using an alternate laptop for this and cannot run the tests. 😞

@tjni
Copy link

tjni commented Jan 2, 2023

I was looking at this too and noticed that importlib is not available in Python 2.7 so the import needs to be conditional if that version still needs to be supported.

Disclaimer: I'm not affiliated with this project so can't facilitate this getting in.

@Andromelus
Copy link
Contributor Author

@tjni Indeed, that may be usefull. I'll have a look into it this week.

Concerning the build fail, checking on Internet, seems like Python 3.6 is not available for the latest version of Ubuntu.

@Andromelus
Copy link
Contributor Author

There are some inconsistent behavior with the tests. Sometimes it works, sometimes it does not (it mostly does not). I'm looking into it.

For example: https://github.com/Andromelus/hdfs/actions/runs/4223653951/jobs/7333640441

@Andromelus
Copy link
Contributor Author

@tjni The test framework only supports Python 3, not bellow. Therefore I will keep the tests on python 3.6 and 3.12.

@Andromelus
Copy link
Contributor Author

Well, more news. fastavro does not support Python 3.12...

@Andromelus
Copy link
Contributor Author

Hello @mtth ,

do you plan to keep maintaining this library ?

Copy link
Owner

@mtth mtth left a comment

Choose a reason for hiding this comment

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

Thank you for the PR @Andromelus and apologies for the very belated review. This importlib change sounds good. A few comments inline.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
CHANGES Outdated Show resolved Hide resolved
hdfs/config.py Outdated Show resolved Hide resolved
@Andromelus
Copy link
Contributor Author

Thank you for the PR @Andromelus and apologies for the very belated review. This importlib change sounds good. A few comments inline.

No problem. Glad to hear back from you.
Will work on it before sunday.

@Andromelus
Copy link
Contributor Author

Changed. Tests passed on my fork.

Copy link
Owner

@mtth mtth left a comment

Choose a reason for hiding this comment

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

Thanks for the update @Andromelus, looks good.

@mtth mtth merged commit cc3f128 into mtth:master Aug 8, 2023
3 checks passed
@musicinmybrain
Copy link
Contributor

This is still broken; there is no importlib.load_source in Python 3.12 either. One other project handled it like this: return42/fspath@f09288e

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.

Deprecation warning: the imp module is deprecated in favour of importlib
4 participants