search

hpcugent / vsc-base

Basic Python libraries used by UGent's HPC group
Other
14 stars 51 forks source link

add configparser as required dependency when using Python 2 #324

Closed wpoely86 closed 2 years ago

wpoely86 commented 2 years ago

@itkovian please have a look.

wpoely86 commented 2 years ago

@boegel This failed on import. Didn't we have a test in vsc-install that checked that all files manage to get imported?

wpoely86 commented 2 years ago

For reason I don't understand test_noshell_async_stdout_glob (test.run.TestRun) is failing even if it's not touched in this PR. Random fluke? I can't restart the tests.

wpoely86 commented 2 years ago

This is a python2 fix. It's fine for me to put it in a dedicated branch python2 only.

boegel commented 2 years ago

@wpoely86 This is doing more than just fixing the import now, was e5dda33 included here by accident?

I can look into merging the import fix (and perhaps add a test to make sure we catch this), but the mail stuff I'm not very familiar with (and it seems like that should go through a separate PR?)

boegel commented 2 years ago

For reason I don't understand test_noshell_async_stdout_glob (test.run.TestRun) is failing even if it's not touched in this PR. Random fluke? I can't restart the tests.

I've restarted the failing test just now.

wpoely86 commented 2 years ago

The fallback thing is python3 only. What e5dda33 does it make sure it works again with both python2 and python3.

stdweird commented 2 years ago

@wpoely86 can you add a unittest so this is tested in both py27 or py3

wpoely86 commented 2 years ago

@stdweird that's the part I totally don't understand. There is a unittest for this. It should have failed under python2, but it doesn't...

A simple python2 -c "from configparser import ConfigParser" correctly fails. Yet if you look at the last run on master in github actions:

2022-04-04T07:29:16.8093706Z test_config_file (test.mail.TestVscMail) ... Reading config file: /home/runner/work/vsc-base/vsc-base/test/data/mailconfig.ini
2022-04-04T07:29:16.8098577Z mail.mail_host: config_host
2022-04-04T07:29:16.8099362Z ok
2022-04-04T07:29:16.8430779Z test_send (test.mail.TestVscMail) ... ok

It's being imported and tested and works...

wpoely86 commented 2 years ago

The tests on #319 should never have passed. Still digging on why they did...

boegel commented 2 years ago

The import works fine with Python 2 as long as you have the configparser Python package installed from PyPI...

wpoely86 commented 2 years ago

https://pypi.org/project/configparser/ is probably what is causing the tests to pass. But it's unclear why it's in our test env

boegel commented 2 years ago

In the test environment, configparser is always installed by vsc-install: https://github.com/hpcugent/vsc-install/blob/8dbc0fbc297d799cd3ff2949716d20751952318b/lib/vsc/install/shared_setup.py#L1483