There is a cyclic import in dmprof

There is a cyclic import in dmprof

Cyclic import (lib.deep_dump -> lib.dump) dues to new pylint checker.

R=kouhei@chromium.org
BUG=439878

[wensheng, 2014-12-08 08:08:53]

On 2014/12/08 08:03:14, wensheng wrote:

As my understand, Dump is base class of DeepDump, but in Dump, it references the
DeepDump to create instance for DumpList.
To fix the cyclic import, I separate DumpList from dump module and let it
directly use DeepDump.

Please check.

[Dai Mikurube (NOT FULLTIME), 2014-12-08 12:35:33]

dmikurube@chromium.org changed reviewers:
+ dmikurube@chromium.org

[Dai Mikurube (NOT FULLTIME), 2014-12-08 12:35:34]

I remember that DeepDump was introduced to support multiple dump formats (like
Android heap dump), but it was not finished.

IIRC, I planned to switch "which loader is called" in Dump.load. It means that
Dump.load creates DeepDump or another SomeDump object based on the file format.
Calling DeepDump.load was not expected. I guess I forgot to remove
DeepDump.load.

Therefore, I gues the right fix is just to remove DeepDump.load. What do you
think?

[Dai Mikurube (NOT FULLTIME), 2014-12-08 12:45:27]

Ah, it doesn't fix the cyclic import error. But, I think I imported DeepDump
from Dump in a very limited scope to limit the impact of this cyclic import.

I'd suggest getting an advice from some Python experts whether it should be
allowed or not. I'm not an expert of Python.

[wensheng, 2014-12-09 02:53:39]

On 2014/12/08 12:45:27, Dai Mikurube (NOT FULLTIME) wrote:
> Ah, it doesn't fix the cyclic import error. But, I think I imported DeepDump
> from Dump in a very limited scope to limit the impact of this cyclic import.
> 
> I'd suggest getting an advice from some Python experts whether it should be
> allowed or not. I'm not an expert of Python.

Hi, Dai,

Sorry to not fix the error.

This error causes my another patch can't commit. 

I try to reproduce the cyclic import error on my local PC (also update
depot_tools), 
but the pylint didn't work same as buildbot, 
I think there are some difference in environment like pylint configuration.

I'm not familiar with Python. 
As my understanding, this cylic import is between dump and deep_dump, 
Dump.load just like creator factory, if separte this function will fix this
error in theory.

Anyway, if there is some other solution to fix (or not fix) this error, I can
take a try.

[Dai Mikurube (NOT FULLTIME), 2014-12-09 03:05:29]

Okay, how about:

1) Remove DeepDump.load.
2) Change "from lib.deep_dump import DeepDump" to "import lib.deep_dump" and
"DeepDump(path, os....) " to "deep_dump.DeepDump(path, os...)". It should be
valid in Python. (I'm not sure how pylint works, though.)
3) If (1) and (2) don't make pylint happy, add "pylint disable" like
http://src.chromium.org/viewvc/chrome/trunk/src/tools/deep_memory_profiler/li...
tentatively with descriptive comments, and do your work.
4) After (3), ask a Python expert about it.

[wensheng, 2014-12-09 03:18:30]

On 2014/12/09 03:05:29, Dai Mikurube (NOT FULLTIME) wrote:
> Okay, how about:
> 
> 1) Remove DeepDump.load.
> 2) Change "from lib.deep_dump import DeepDump" to "import lib.deep_dump" and
> "DeepDump(path, os....) " to "deep_dump.DeepDump(path, os...)". It should be
> valid in Python. (I'm not sure how pylint works, though.)
> 3) If (1) and (2) don't make pylint happy, add "pylint disable" like
>
http://src.chromium.org/viewvc/chrome/trunk/src/tools/deep_memory_profiler/li...
> tentatively with descriptive comments, and do your work.
> 4) After (3), ask a Python expert about it.

OK, I'll try to fix this error. It maybe need some time.
Thanks.