Fix exception when toml lib is not installed #2506
Conversation
luigi/configuration/core.py
Outdated
| "Please, install luigi with required parser:\n" | ||
| "pip install luigi[{parser}]" | ||
| ) | ||
| raise ImportError(msg.format(parser=parser)) |
There was a problem hiding this comment.
This if not parser_class.enabled is checked at least twice here. Can that be extracted into a method or something for DRY programming purposes?
| "Set up right parser via env var: " | ||
| "export LUIGI_CONFIG_PARSER={added}" | ||
| ) | ||
| warnings.warn(msg.format(added=parser, used=PARSER)) |
There was a problem hiding this comment.
Does warnings.warn() also write to logs?
There was a problem hiding this comment.
I use it because warnings.warn used in many other places in project. Yes, warnings can be captured.
|
Can someone fix the TravisCI? It has troubles with AWS keys. It's something related to #2171? |
@dlstadther do you know why there's so many crashes (does the tests rely on external services?). |
| msg = ( | ||
| "Parser not installed yet. " | ||
| "Please, install luigi with required parser:\n" | ||
| "pip install luigi[{parser}]" |
There was a problem hiding this comment.
This can only be toml right? It's fine to hard code it too.
|
@Tarrasch This AWS client failure just started. I haven't been able to invest the time yet to determine what the issue is. I know we mock boto with moto. But when last updated our moto version, things were still ok. Unless a minor version broke this. |
|
moto version between last passing build and current is the same. However, boto3 increased from 1.7.84 to 1.8.2 and botocore from 1.10.84 to 1.11.2 |
|
Builds are green again. I suspect that the boto issue we were experiencing was a Travis issue. |
* upstream-master: Fix S3Client.copy return value consistency (spotify#2488) s3client check for deprecated host keyword and raise error with the details (spotify#2493) Fix exception when toml lib is not installed (spotify#2506) Add Okko to companies that use luigi (spotify#2512) Added optional choice for hdfs clients (spotify#2487) Version 2.7.8 revert tornado upgrade Version 2.7.7 added a new event 'progress' (spotify#2498) Add Uppsala University / pharmb.io as user (spotify#2496)
|
Thank you ^^ |
Description
Fix issue from comments to #2457 and some related stuff:
LUIGI_CONFIG_PATHwith*.tomlfile is presented in env vars, buttomllib is not installed yet.ImportErrorinstead of simple logging, because this is critical issue, that doesn't allow lib works fine.LUIGI_CONFIG_PATHandLUIGI_CONFIG_PARSERpoint to different parsers.installationsection.Have you tested this? If so, how?
Some minimal tests included and works fine.