fix crash in get_config_dict when copying modules that were imported in easyconfig file (like 'import os')

[damianam]

can't be serialized. For instance os, if it is imported in a eb file

edit: fix for traceback:

Traceback (most recent call last):
  ...
  File "/Volumes/work/easybuild-framework/test/framework/easyconfig.py", line 4449, in test_easyconfig_import
    ec = EasyConfig(test_ec)
  File "/Volumes/work/easybuild-framework/easybuild/framework/easyconfig/easyconfig.py", line 514, in __init__
    self.parse()
  File "/Volumes/work/easybuild-framework/easybuild/framework/easyconfig/easyconfig.py", line 698, in parse
    ec_vars = self.parser.get_config_dict()
  File "/Volumes/work/easybuild-framework/easybuild/framework/easyconfig/parser.py", line 222, in get_config_dict
    cfg = self._formatter.get_config_dict()
  File "/Volumes/work/easybuild-framework/easybuild/framework/easyconfig/format/one.py", line 139, in get_config_dict
    if key != '__builtins__' and "'module'" not in type(cfg[key]):
TypeError: argument of type 'type' is not iterable

[boegel]

@damianam We should have an enhanced test for this (I can look into that).

I also wonder if we can come up with a better fix... I guess the failure was trigger by trying to pickle os itself?

[damianam]

Yes, exactly.

[boegel]

I'm happy to follow up on this myself with a test + making the requested changes (and leave merging the PR to someone else)

changes made

[smoors]

maybe also skip class objects?

... and "class 'type'" not in str(type(cfg[key])) and "type 'classobj'" not in str(type(cfg[key])):

[boegel]

When would we ever have a class definition in easyconfigs though?!

[smoors]

When something is possible, there will always be one person one day that will do it ;)

[boegel]

Well sure, but there's all kinds of weird stuff you could in theory do in an easyconfig file, since it's essentially Python code. That doesn't mean we support all those things.

In 558829b I've made sure you don't get an ugly traceback with a funky easyconfig file, but a hopefully useful clean error message.

[smoors]

that's perfectly fine, and I like that users will get a nice error message.

to be clear, we should indeed not support all possible funky stuff, but there's always a balance between what's useful to support and what can be implemented easily.
in this case the balance was not so clear-cut to me. some users prefer to put all logic into the easyconfig instead of the easyblock, so that could definitely be a useful use case (but again, I'm not saying we should support it)

[damianam]

Just to add a little bit of context: We are starting to rely on import os to have "smart" easyconfigs that do different things depending on the system where they are being installed. I would pretty much like to keep this working in the future 😅 . There are of course other ways to do the same thing (hooks, easyblocks, overlays with slightly different EB files), but this is pretty convenient in some cases.

[smoors]

lgtm

[smoors]

lgtm

[smoors]

Going in, thanks @damianam!