Fix problem with dummy configuration file on ReadTheDocs #3579
Conversation
|
With this fix it seems to build again: https://readthedocs.org/projects/aiida-core/builds/10026000/ |
| if IN_RT_DOC_MODE: | ||
| # The following is a dummy config.json configuration that it is used for the | ||
| # proper compilation of the documentation on readthedocs. | ||
| from aiida.manage.external.postgres import DEFAULT_DBINFO | ||
| return Config( | ||
| '/dev/null', { | ||
| filepath, { |
There was a problem hiding this comment.
Ah hm - I wonder: What if now someone sets the environment variable to run in RTD mode on their local laptop. Will this overwrite the existing config file?
Perhaps it would be safer to simply use some temporary file path?
Could we perhaps reuse some of the code from here:
There was a problem hiding this comment.
That's what I originally was trying as well, but then it is difficult to control the lifetime of it. I will have to create the tempfile in the load_config function, but then cannot control when it should be deleted. This way we will end up with uncleaned temp files. Frankly, I wanted to get rid of these hacks just for RTD for a long time. I remember you also opened an issue recently with a request for some official API to do what we currently do in the docs/source/conf.py for I believe plugins to be able to do the same. I'd rather have this go in now and then fix it properly soon
There was a problem hiding this comment.
Ok, fine not to merge the functionality.
I still think, though, that we should be sure not to overwrite an existing config file (creating a temporary one without deleting it is preferable).
Are we sure your approach does not overwrite an existing config file?
The recent change in the backup behavior of the configuration file leads to problems on ReadTheDocs. The dummy config returned by `load_config` when on RTD uses `/dev/null` as dummy filepath, but the new backup mechanism will try to write a backup using that filepath as a prefix, leading to a filepath that can't be written to. The temporary fix is to use an actual temporary file for the dummy filepath.
sphuber
force-pushed
the
fix_3578_readthedocs
branch
from
451b448 to
3d75f9a
Compare
|
@ltalirz OK I replaced it with a real temporary file, but it is not cleaning it up. |
|
Hacky solution: why not put it in the sphinx BUILDDIR? Then This probably doesn't matter much if the root issue will be fixed properly soon, though. |
Fixes #3578
The recent change in the backup behavior of the configuration file leads
to problems on ReadTheDocs. The dummy config returned by
load_configwhen on RTD uses
/dev/nullas dummy filepath, but the new backupmechanism will try to write a backup using that filepath as a prefix,
leading to a filepath that can't be written to. The temporary fix until
we get rid of this hacky dummy profile concept is to simply use the
default location of the config file.