Port Weboob to KF5
ClosedPublic

Authored by wojnilowicz on Jan 13 2018, 12:17 PM.

Details

Summary

Kross doesn't want to run python scripts anymore, so Weboob plugin is dead. This patch makes it alive again.

Changes:

  1. no KF5::Kross dependency, as it is deprecated,
  2. new Python2 dependency, as it's all we need to run weboob,
  3. if no weboob is installed then plugin won't be compiled neither,
  4. renamed weboob.py to kmymoneyweboob.py and put it in kmymoney/weboob directory to avoid name clashes,
  5. renamed class names to better represent what they are for,
  6. introduced little exception handling from python scripts and informational dialogs for that,
  7. made single private class out of two in mapaccount, it is mystery why two of them were needed before,
  8. avoided crashes/never ending progress bars if no weboob backends/accounts were loaded.
Test Plan

Tested backends only, as I have no account in banks supported by weboob.

Diff Detail

Repository
R261 KMyMoney
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
wojnilowicz requested review of this revision.Jan 13 2018, 12:17 PM
wojnilowicz created this revision.

In case I install the patch, weboob is disabled apparently because I am missing a few things here on my system. So all I can say is that KMyMoney compiles and work in case the prerequisites for weboob are not met.

In case I install the patch, weboob is disabled apparently because I am missing a few things here on my system. So all I can say is that KMyMoney compiles and work in case the prerequisites for weboob are not met.

Could you check whether you're missing python dependency or weboob?
In case of weboob it's very easy when you follow user installation from git by following command

./tools/local_install.sh ~/.local/bin
conet accepted this revision as: conet.Jan 23 2018, 3:53 PM
This revision is now accepted and ready to land.Jan 23 2018, 3:53 PM
pino requested changes to this revision.Feb 8 2018, 8:26 AM
pino added a subscriber: pino.

Considering Python2 has been on maintainance mode for a number of years already, and it will be EOL in 2020 (see https://pythonclock.org/), then please use Python3 directly (or at least allow to use both Python2 and Python3).

CMakeLists.txt
176–177

Looking at build time for a Python module used at runtime does not make much sense to me. This rather ought to be some kind of runtime check when loading the weboob plugin.

kmymoney/plugins/weboob/CMakeLists.txt
2

why?

kmymoney/plugins/weboob/dialogs/CMakeLists.txt
24

alkimia seems not needed

kmymoney/plugins/weboob/interface/CMakeLists.txt
19

alkimia seems not needed

kmymoney/plugins/weboob/interface/weboobinterface.cpp
47–52

Is this hack needed for real? What does it happen if it is not done?

61

toLocal8Bit() is not correct for paths, please use QFile::encodeName()

84

toLatin1() if they are strings, or again QFile::encodeName() for paths. (This applies also to the other occurrences of toLocal8Bit().)

This revision now requires changes to proceed.Feb 8 2018, 8:26 AM
This revision was not accepted when it landed; it landed in state Needs Revision.Feb 8 2018, 3:09 PM
This revision was automatically updated to reflect the committed changes.
In D9855#202891, @pino wrote:

Considering Python2 has been on maintainance mode for a number of years already, and it will be EOL in 2020 (see https://pythonclock.org/), then please use Python3 directly (or at least allow to use both Python2 and Python3).

I have a feeling you didn't compile the code, so you don't know what you're talking about. Please apply your advices, try to compile and return with new look :)

kmymoney/plugins/weboob/CMakeLists.txt
2

I have no idea why it was here, but I left it just in case.

kmymoney/plugins/weboob/interface/weboobinterface.cpp
47–52

Have you read the bug report? I found it based on the same symptoms.

pino added a comment.Feb 8 2018, 7:34 PM
In D9855#202891, @pino wrote:

Considering Python2 has been on maintainance mode for a number of years already, and it will be EOL in 2020 (see https://pythonclock.org/), then please use Python3 directly (or at least allow to use both Python2 and Python3).

I have a feeling you didn't compile the code, so you don't know what you're talking about.

Looking at the patch was enough to say that your code requires Python2 and actively rejects Python3; building confirms that:

-- Found PythonInterp: /usr/bin/python3.6 (found suitable version "3.6.4", minimum required is "2.6") 
-- Found PythonLibs: /usr/lib/x86_64-linux-gnu/libpython3.6m.so (found suitable version "3.6.4", minimum required is "3.6.4") 
CMake Warning at CMakeLists.txt:192 (message):
  Python 2 required, but Python 3 found.

[...]
weboob plugin:                           no

The rest of my notes still applies.

Please apply your advices, try to compile and return with new look :)

Just a note about this (it will be my only one): since we never interacted before, nor we know each other, this kind of sentence is not exactly polite (it goes on the borderline of rudeness). I assume that this was surely not your intention, so my suggestion is to not use such approach with newcomers (or people you did not interact already).

kmymoney/plugins/weboob/CMakeLists.txt
2

Small hint: do not include code without knowing what it does.

kmymoney/plugins/weboob/interface/weboobinterface.cpp
47–52

Yes, I read it, and it is not exactly the same thing. If, and only if, there will be similar reports in the future, then the workaround coud be taken into account. Small hint: do not get wet before it is actually raining.

In D9855#203231, @pino wrote:
In D9855#202891, @pino wrote:

Considering Python2 has been on maintainance mode for a number of years already, and it will be EOL in 2020 (see https://pythonclock.org/), then please use Python3 directly (or at least allow to use both Python2 and Python3).

I have a feeling you didn't compile the code, so you don't know what you're talking about.

Looking at the patch was enough to say that your code requires Python2 and actively rejects Python3; building confirms that:

Why would I need Python 3 if I need Python 2 explicitly? Python 3 is completely useless in my case. I don't understand your point.

The rest of my notes still applies.

Alkimia is needed (otherwise failed compilation), hack is needed. Encoding will be fixed.

Please apply your advices, try to compile and return with new look :)

Just a note about this (it will be my only one): since we never interacted before, nor we know each other, this kind of sentence is not exactly polite (it goes on the borderline of rudeness). I assume that this was surely not your intention, so my suggestion is to not use such approach with newcomers (or people you did not interact already).

Sorry if you felt offended. It's true, that it was not my intention. English is not my native language. As you could see I tried to write polite sentence. I don't know what I can do about it.

kmymoney/plugins/weboob/CMakeLists.txt
2

I actually commented it out, as I don't need it.

kmymoney/plugins/weboob/interface/weboobinterface.cpp
47–52

Yes, I read it, and it is not exactly the same thing. If, and only if, there will be similar reports in the future, then the workaround coud be taken into account.

Without this code it's not working on Fedora, as it was described on bugzilla. I don't see why it's not exactly the same thing. The bug is old and because of that I see no hope for fixing it.

I don't understand your point. What would you do differently here?

Small hint: do not get wet before it is actually raining.

I don't get it, as English is not my native language.

pino added a comment.Feb 15 2018, 7:06 AM
In D9855#203231, @pino wrote:
In D9855#202891, @pino wrote:

Considering Python2 has been on maintainance mode for a number of years already, and it will be EOL in 2020 (see https://pythonclock.org/), then please use Python3 directly (or at least allow to use both Python2 and Python3).

I have a feeling you didn't compile the code, so you don't know what you're talking about.

Looking at the patch was enough to say that your code requires Python2 and actively rejects Python3; building confirms that:

Why would I need Python 3 if I need Python 2 explicitly? Python 3 is completely useless in my case. I don't understand your point.

This is exactly what I mentioned in my initial comment:Python 2 will be dead soon, in approximately 2 years. As consequence, introducing right now Python code that does not work with Python 3 is not a good idea, as requires already the need for porting, in case it is not a temporary sort-term solution.
Almost all the upstreams of Python projects already ported to Python 3 their code, and Linux (and not only) distributions are already looking into switching to Python 3 as default (some already did, like Archlinux and Fedora), and possibly to not even install Python 2 in base/desktop installations.

I hope this explains why it is really important to not have Python 2-only code around.

The rest of my notes still applies.

Alkimia is needed (otherwise failed compilation), hack is needed. Encoding will be fixed.

I do not see any Alkimia class used in the weboob plugin. Can you please specify which issues do you get?

Please apply your advices, try to compile and return with new look :)

Just a note about this (it will be my only one): since we never interacted before, nor we know each other, this kind of sentence is not exactly polite (it goes on the borderline of rudeness). I assume that this was surely not your intention, so my suggestion is to not use such approach with newcomers (or people you did not interact already).

Sorry if you felt offended. It's true, that it was not my intention. English is not my native language. As you could see I tried to write polite sentence. I don't know what I can do about it.

English is not my native language either. OTOH your tone was, and still is, passive-aggressive.

In D9855#206600, @pino wrote:
In D9855#203231, @pino wrote:
In D9855#202891, @pino wrote:

Considering Python2 has been on maintainance mode for a number of years already, and it will be EOL in 2020 (see https://pythonclock.org/), then please use Python3 directly (or at least allow to use both Python2 and Python3).

I have a feeling you didn't compile the code, so you don't know what you're talking about.

Looking at the patch was enough to say that your code requires Python2 and actively rejects Python3; building confirms that:

Why would I need Python 3 if I need Python 2 explicitly? Python 3 is completely useless in my case. I don't understand your point.

This is exactly what I mentioned in my initial comment:Python 2 will be dead soon, in approximately 2 years. As consequence, introducing right now Python code that does not work with Python 3 is not a good idea, as requires already the need for porting, in case it is not a temporary sort-term solution.
Almost all the upstreams of Python projects already ported to Python 3 their code, and Linux (and not only) distributions are already looking into switching to Python 3 as default (some already did, like Archlinux and Fedora), and possibly to not even install Python 2 in base/desktop installations.

I hope this explains why it is really important to not have Python 2-only code around.

I agree that Python 2 is not a good idea, but I think there are some points you're missing:

  1. 2 years is still very long term,
  2. Fedora still uses Python 2 as the default one,
  3. Weboob depends on Python 2 and it's not the utility that I maintain.

The rest of my notes still applies.

Alkimia is needed (otherwise failed compilation), hack is needed. Encoding will be fixed.

I do not see any Alkimia class used in the weboob plugin. Can you please specify which issues do you get?

You don't see it and I don't see it too, but did you actually compiled it? I assume not. I compiled it and it screams for alkimia in some dependent header.
It may be that alkimia is faulty.

Please apply your advices, try to compile and return with new look :)

Just a note about this (it will be my only one): since we never interacted before, nor we know each other, this kind of sentence is not exactly polite (it goes on the borderline of rudeness). I assume that this was surely not your intention, so my suggestion is to not use such approach with newcomers (or people you did not interact already).

Sorry if you felt offended. It's true, that it was not my intention. English is not my native language. As you could see I tried to write polite sentence. I don't know what I can do about it.

English is not my native language either. OTOH your tone was, and still is, passive-aggressive.

Maybe that's only your feeling. I think it's not the right place to discuss such topics here.