Fix circular imports

We have circular imports disabled by default with the new linting.

Unfortunately these seem to have crept in since we have not yet fixed #146 (closed)

This is messy, and worse; it is simply not supported in all versions of python we support.

Marking this as critical since BuildStream is completely unusable on LTS distros where only 3.4 is available.

added High_impact Important To Do and removed Critical labels

marked this issue as related to #146 (closed)

@tristanvb is this still an issue? I ask this since #146 (closed) is closed now and we should be testing against python 3.4.

I've just disabled the warning for this and it seems to me that the only current occurrence of this is here: https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/_frontend/app.py#L139

Note that linuxapp.py imports the App class from the app.py module.

Let's close this as the debian 8 tests are passing with python 3.4.

closed

@tristanvb, I would have thought that the closing of this issue would have come along with a patch which both enables pylint's cyclic-import warning and deals with any circular imports?

We still have an instance of a cyclic import which I have linked to in the comment above.

To be clear, does the closing of this issue mean that we intend to ignore this one instance and keep the pylint warning disabled?

@jennis: I would like to enable the pylint warning if possible.

@tristanvb does this mean we should open a different ticket or that this ticket should still be opened to consider @jennis case?

Meanwhile I will put it in Review state.

reopened

added Verify label

@tristanvb, I would have thought that the closing of this issue would have come along with a patch which both enables pylint's cyclic-import warning and deals with any circular imports?

That's one way to look at it, but I opened this issue with the understanding that python 3.4 just does not support circular imports; the test cases running python 3.4 are proving that to be incorrect - as such it's not really a problem anymore.

This does not mean that it is not messy to have circular imports: it's still desirable to change that - whether we want to track progress towards cleaning up this particular messyness in an issue, is another question (but rather tedious, it's now become a fairly minor stylistic point, nothing prevents it from being fixes without having an issue especially open for it).

@tristanvb does this mean we should open a different ticket or that this ticket should still be opened to consider @jennis case?

Meanwhile I will put it in Review state.

@toscalix: there are two options.

• Turn this issue into an issue detailed like #320, i.e. an issue about enabling a linter check
  • In the same vein, we could close this issue and open something like #320 but specifically about circular imports
• Just leave this issue closed, accepting that the severity/impact has become low enough that we don't really need a record of it.

removed Important label

removed To Do label

@toscalix: there are two options.

• Turn this issue into an issue detailed like #320, i.e. an issue about enabling a linter check

  • In the same vein, we could close this issue and open something like #320 but specifically about circular imports

• Just leave this issue closed, accepting that the severity/impact has become low enough that we don't really need a record of it.

I will go for the latter.

closed