Project

General

Profile

Bug #15260

ceph-disk and ceph-detect-init builds do check-only things on every build

Added by Dan Mick over 4 years ago. Updated 8 months ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
-
Target version:
-
% Done:

0%

Source:
Development
Tags:
Backport:
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Pull request ID:
Crash signature:

Description

It seems that ceph-disk and ceph-detect-init's Makefiles always create the virtualenv, whether called during make or make check, but it seems that it only needs the virtualenv for testing (when called during check).

I'm not 100% certain of the fix here but it would be nice if only the ceph-disk-install-data target were run on non-check builds.

We noticed this when the virtualenv construction was failing because of long paths and #! truncation, which prompted the question "why is a non-check build even doing this?"

History

#1 Updated by Dan Mick over 4 years ago

  • Assignee set to Loic Dachary

Loic, if you could clarify intent and requirements, I'm not necessarily asking you to fix it, but assigning just to get your attention on it

#2 Updated by Loic Dachary over 4 years ago

We're not currently using virtualenv during make. But we should, at least for tox -e flake8. I think removing the build dependency to virtualenv would go in the wrong direction. If anything we'll need virtualenv more and more moving foward. Not less and less.

#3 Updated by Dan Mick over 4 years ago

I'm not saying to remove the dependency; I'm saying that it's not necessary outside the test build.

#4 Updated by Dan Mick over 4 years ago

(and even more so that it runs every build, whether anything's changed or not)

#5 Updated by Loic Dachary over 4 years ago

I think we should run flake8 on build.

#6 Updated by Dan Mick over 4 years ago

I would support that enhancement

#7 Updated by Ken Dreyer over 4 years ago

It's fine to run flake8 on build, but can we not use tox or a virtualenv, and use python-flake8 from the distro instead?

That would reduce the complexity of the build considerably.

#8 Updated by Loic Dachary almost 4 years ago

  • Assignee deleted (Loic Dachary)

#9 Updated by Alfredo Deza almost 4 years ago

It is not necessary to install tox to run flake8. This can easily be done with the flake8 system package.

Running virtualenv everywhere every time is a burden

#10 Updated by Ian Colle almost 4 years ago

  • Assignee set to Loic Dachary

Please create an RFE to run flake8 on builds, but stop creating the virtualenv for non-test builds.

#11 Updated by Loic Dachary almost 4 years ago

  • Status changed from New to Fix Under Review

Here is a tentative implementation, please let me know what you think: https://github.com/ceph/ceph/pull/11177

#12 Updated by Loic Dachary over 3 years ago

  • Status changed from Fix Under Review to 12
  • Assignee deleted (Loic Dachary)

#13 Updated by Ian Colle over 3 years ago

  • Assignee set to Loic Dachary

#14 Updated by Loic Dachary over 3 years ago

  • Assignee deleted (Loic Dachary)

I don't know how to fix this, someone else will have to work on it.

#15 Updated by Patrick Donnelly 8 months ago

  • Status changed from 12 to New

Also available in: Atom PDF