Print warning to stdout on load_profile for development versions
#5311
Conversation
| version = parse(__version__) | ||
|
|
||
| if version.is_postrelease: | ||
| echo.echo_warning(f'You are currently using a post release development version of AiiDA: {version}') |
There was a problem hiding this comment.
This should use logger warnings, rather than using click outside of the CLI
There was a problem hiding this comment.
It does use the logging system though. It is true that the module is in the aiida.cmdline module, but after the recent refactor, they all go through the logger.
I agree, but last time I discussed this, @giovannipizzi had reservations about this. I am happy to implement it, but before I do, I would like consensus that we want this. So let's first get opinions from the rest. |
Codecov Report
@@ Coverage Diff @@
## develop #5311 +/- ##
===========================================
- Coverage 82.01% 81.91% -0.10%
===========================================
Files 533 530 -3
Lines 38243 38019 -224
===========================================
- Hits 31363 31140 -223
+ Misses 6880 6879 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| if version.is_postrelease: | ||
| echo.echo_warning(f'You are currently using a post release development version of AiiDA: {version}') | ||
| echo.echo_warning('Be aware that this is not recommended for production and is not officially supported.') | ||
| echo.echo_warning('Databases used with this version may not be compatible with future releases of AiiDA.\n') |
There was a problem hiding this comment.
Maybe say explicitly 'and you might not be able to migrate your data to a production version of AiiDA'? Or something like this?
|
Regarding the configuration option: if you feel it's useful for developers, feel free to add it. So, if you want to add the option (keeping the message as it is, apart from my other comment) feel free to go ahead! |
sphuber
force-pushed
the
fix/5290/load-profile-check-version
branch
2 times, most recently
from
cbf207f to
850fb22
Compare
|
@chrisjsewell @giovannipizzi I have implemented the option with a test and updated the warning message. Should be good for final review |
The `load_profile` method now calls a `check_version` function. This checks what version of `aiida-core` is currently installed and if it is a post release development version, a warning is printed that it should not be run with production databases. The reason is that we cannot support automatic data migrations for anything other than released versions (alpha, beta, and pre-releases are supported). This function relies on the protocol that development branches will always use a proper post release development version defined in the `__version__` attribute of the init file of the `aiida` module. This means it should be of the form `'2.0.0.post0`. The choice to place the check of the version in `load_profile` is based on two requirements: * It should be guaranteed to run however the database can be loaded * It should ideally only run once and not spam stdout multiple times The `load_profile` function is the only way to load a profile which is a necessary step to load any database, therefore this should guarantee that the version check is always called when it is needed. Another location could have been `Manager._load_backend` which is also guaranteed to be called before a database is loaded, however, the downside here is that this can be called multiple times. this would lead to the warning being spammed multiple times. A configuration option `warning.development_version` is added. It is global only and by default set to true. When set to false, the warning will no longer be displayed.
sphuber
force-pushed
the
fix/5290/load-profile-check-version
branch
from
850fb22 to
25fe544
Compare
Fixes #5290
The
load_profilemethod now calls acheck_versionfunction. Thischecks what version of
aiida-coreis currently installed and if it isa post release development version, a warning is printed that it should
not be run with production databases. The reason is that we cannot
support automatic data migrations for anything other than released
versions (alpha, beta, and pre-releases are supported).
This function relies on the protocol that development branches will
always use a proper post release development version defined in the
__version__attribute of the init file of theaiidamodule. Thismeans it should be of the form
2.0.0.post0.The choice to place the check of the version in
load_profileis basedon two requirements:
The
load_profilefunction is the only way to load a profile which isa necessary step to load any database, therefore this should guarantee
that the version check is always called when it is needed. Another
location could have been
Manager._load_backendwhich is alsoguaranteed to be called before a database is loaded, however, the
downside here is that this can be called multiple times. this would lead
to the warning being spammed multiple times.