Can't generate a new config_dump.json if the old config_dump.json is bad
Reported by
jrbarnette@chromium.org,
Dec 1 2017
|
|||||
Issue description
Inside the chromite repository, the following command sequence fails:
$ rm cbuildbot/config_dump.json
$ bin/cbuildbot_view_config --update_config
The error is this Python exception:
Traceback (most recent call last):
File "bin/cbuildbot_view_config", line 77, in <module>
from chromite.lib import commandline
File "/usr/local/google/home/jrbarnette/repos/cros.base/chromite/lib/commandline.py", line 29, in <module>
from chromite.lib import gs
File "/usr/local/google/home/jrbarnette/repos/cros.base/chromite/lib/gs.py", line 30, in <module>
from chromite.lib import path_util
File "/usr/local/google/home/jrbarnette/repos/cros.base/chromite/lib/path_util.py", line 16, in <module>
from chromite.lib import git
File "/usr/local/google/home/jrbarnette/repos/cros.base/chromite/lib/git.py", line 27, in <module>
site_config = config_lib.GetConfig()
File "/usr/local/google/home/jrbarnette/repos/cros.base/chromite/lib/config_lib.py", line 1938, in GetConfig
_CACHED_CONFIG = LoadConfigFromFile(filename)
File "/usr/local/google/home/jrbarnette/repos/cros.base/chromite/lib/config_lib.py", line 1798, in LoadConfigFromFile
json_string = osutils.ReadFile(config_file)
File "/usr/local/google/home/jrbarnette/repos/cros.base/chromite/lib/osutils.py", line 165, in ReadFile
with open(path, mode) as f:
IOError: [Errno 2] No such file or directory: '/usr/local/google/home/jrbarnette/repos/cros.base/chromite/cbuildbot/config_dump.json'
Basically, in 'lib/git.py', there's this assignment at module level:
site_config = config_lib.GetConfig()
If the call to `config_lib.GetConfig()`, then importing the module
will fail, and if the import fails, you can't generate a new config
file to fix the problem. http://shortn/_p7eNySFSLm
,
Dec 4 2017
I've been meaning to fix this for a long time. Thanks!
,
Dec 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/d033328a4fb27071f96352739f4e47ac339520cb commit d033328a4fb27071f96352739f4e47ac339520cb Author: Allen Li <ayatane@chromium.org> Date: Tue Dec 05 04:02:38 2017 git: Dont load config at import time This import time call causes generation of the config_dump.json to fail if the existing file cannot be loaded, or if it is missing. BUG=chromium:790823 TEST=rm cbuildbot/config_dump.json TEST=bin/cbuildbot_view_config --update_config Change-Id: I326a23c8a075dbbcda860b198388a263aabdd035 Reviewed-on: https://chromium-review.googlesource.com/802616 Commit-Ready: Allen Li <ayatane@chromium.org> Tested-by: Allen Li <ayatane@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/d033328a4fb27071f96352739f4e47ac339520cb/lib/git.py
,
Dec 5 2017
,
Dec 5 2017
Let's not declare victory just yet. The specific design feature that made this possible was the call to config_lib.GetConfig() at module level. There are 35 such calls still present in the source base. It would be relatively easy to change an import such that one of those 35 calls re-appears in the imports needed for generating the JSON file. That would put us right back where we were. So, we need a plan for how we'll quit relying on those module-level calls.
,
Dec 6 2017
Sounds like a bug "Don't do this thing at import time", which is a subset of the bug "Don't do all of the bad things at import time" that is common across Autotest and chromite. The title of this bug addressed only the immediate config_dump issue. We should at least rename the bug or create a new one to track the problem explicitly.
,
Dec 6 2017
Just a note, the import time site_config creation is mostly my fault. It was intentionally done as a performance improvement that has long been irrelevant. Cleaning them up is annoying, but not hard.
,
Mar 30 2018
,
Mar 30 2018
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by ayatane@chromium.org
, Dec 1 2017Status: Started (was: Available)