search

src-d / style-analyzer

Lookout Style Analyzer: fixing code formatting and typos during code reviews
GNU Affero General Public License v3.0
32 stars 21 forks source link

Use automatic args erase feature from modelforge logging #713

Closed irinakhismatullina closed 5 years ago

irinakhismatullina commented 5 years ago

Use new modelforge.slogging.add_logging_args() parameter: erase_args=True

vmarkovtsev commented 5 years ago

Build failure

irinakhismatullina commented 5 years ago

@vmarkovtsev This happens because the code now is incompatible with latest changes in lookout-sdk-ml. So should I use the old version, and explicitly demand new version for modelforge?

vmarkovtsev commented 5 years ago

@irinakhismatullina According to the CI logs, the reason is different, please fix the tests.

vmarkovtsev commented 5 years ago

You can hack .travis.yml and Dockerfile to force the right packages anyway

irinakhismatullina commented 5 years ago

Well this is definitely related to the newest changes in lookout-sdk-ml: https://travis-ci.com/src-d/style-analyzer/jobs/186382553#L1427 And this was also changed in the last release https://travis-ci.com/src-d/style-analyzer/jobs/186382553#L1435

The question is: do i need roll back to lookout-sdk-ml==0.15.0 as it were before (because clearly no work was done through the format (and maybe typos too) analyzer to support the updates?

irinakhismatullina commented 5 years ago

The problem here is sourced-ml requirements for modelforge<12.0. The same happens during local default package installation. So what's the best way to deal with this situation @vmarkovtsev ? Add modelforge==12.0 specifically to the requirements?

zurk commented 5 years ago

I close this one because it is being taken over by https://github.com/src-d/style-analyzer/pull/719

vmarkovtsev commented 5 years ago

@zurk the second case is not covered in 719

vmarkovtsev commented 5 years ago

@irinakhismatullina Can you please rebase

zurk commented 5 years ago

One test in docker is failed:

======================================================================
ERROR: test_handlers (lookout.style.format.tests.test_main.MainTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/style-analyzer/lookout/style/format/tests/test_main.py", line 37, in test_handlers
    parser = main.create_parser()
  File "/style-analyzer/lookout/style/format/cmdline_tools.py", line 104, in create_parser
    slogging.add_logging_args(parser, erase_args=True)
TypeError: add_logging_args() got an unexpected keyword argument 'erase_args'
----------------------------------------------------------------------

because of wrong modelforge version. Version conflict fun continues.

vmarkovtsev commented 5 years ago

@zurk I know why. Because

Installing collected packages: lookout-style, modelforge, pyspark
  Running setup.py develop for lookout-style
  Found existing installation: modelforge 0.12.0
    Uninstalling modelforge-0.12.0:
      Successfully uninstalled modelforge-0.12.0
Successfully installed lookout-style modelforge-0.11.1 pyspark-2.2.1

👿 🔥 😠

vmarkovtsev commented 5 years ago

Should be fixed in https://github.com/src-d/style-analyzer/pull/727

vmarkovtsev commented 5 years ago

The tests have passed, but not for the right reason hehe