sekruse / metanome-cli

Run Metanome algorithms from the command line
http://www.metanome.de/
Apache License 2.0
7 stars 6 forks source link

Bugfix DB connections and support all requirement types #5

Closed f4lco closed 6 years ago

f4lco commented 6 years ago

For details, see the individual commit messages. Main selling points are fixing a lot of shortcomings of the previous approach on algorithm configuration, also applying default values and fixing issue #1. I believe below changes make metanome-cli more robust. For instance the user should be warned in case the algorithm is only partially configured, for instance. Also other frequent misusage patterns should cause metanome-cli to fail loudly.

sekruse commented 6 years ago

Thanks for the PR, f4lco, and sorry for not coming back earlier. I will merge this now, but let me point out a couple of issue FTR:

f4lco commented 6 years ago

Thank you for your time, @sekruse! Time was not an issue, since I was unsure how many bugfixes / nice-to-haves would come up during the course of the seminar. Again thanks for the feedback - I do appreciate to have heard back from you. Yes, this has been an accumulate PR, which is not ideal. However, there a couple commits with detailed messages explaining the change, and formatting has been local to one of these. Concerning logging, I believe in the fact that algorithms should never have to ship a logging implementation / configuration - that is a task better left to the host environment, let it be the original Metanome backend or Metanome CLI or the test runner. To my opinion algorithms should log using slf4j, which decouples logging implementation from API.