Hi Piotr, I'm at French course today from 11-13, so I can stop by your office before or after.
On 15 Nov 2012, at 21:40, Piotr Praczyk <piotr.prac...@cern.ch> wrote: > Just a small amendment to the previous e-mail > > I have just realised that in the 2nd question, the logical solution would be > in every commit consider present scripts as applied... which basically means > that inveniocfg --create-tables should always mark all upgrade scripts as > applied (this seems not to happen) > I misunderstand something or this is a bug ? No, you're right. There's an currently an inconsistency between tabcreate.sql, inveniocfg --create-table and --upgrade. I'll try to see if I can get a fix in today. The tables created in the upgrader scripts, are currently also created in tabcreate.sql, so the correct would be to have them marked as alreday applied. > Piotr > ________________________________________ > From: Piotr Praczyk > Sent: 15 November 2012 21:37 > To: project-invenio-devel (Invenio developers mailing-list); > inspire-...@slac.stanford.edu > Subject: Upgrader script > > Hello > > > I have several questions concerning the Upgrader mechanism introduced to > Invenio. > > First of all, could someone who knows the topic better, review my upgrate > module ? It can be seen under: > > http://invenio-software.org/repo/personal/invenio-ppraczyk/commit/?h=bibdocfile-squashed-ultimate-rebased&id=be09d38416fe5adc0898f40e94fe80118dedd70e > from invenio.config import CFG_LOGDIR, CFG_SITE_SUPPORT_EMAIL, > CFG_BIBDOCFILE_FILEDIR, CFG_BIBDOCFILE_FILESYSTEM_BIBDOC_GROUP_LIMIT +from invenio.bibdocfile import BibRecDocs, InvenioBibDocFileError import os, sys I guess they are related to your question below, but in the commit they are not used, and should thus not be imported. > from invenio import bibdocfile Any non-safe import, should be imported in one of the methods instead of in the module. In the method you can catch an possible import errors as well, and raise a RuntimeError. This is to ensure that the module can be loaded even prior to Invenio being installed (when running make check-upgrade). In the do_upgrade you can try to import it, catch possible ImportError and bail out via RuntimeError if it fails. Logging: logger = logging.getLogger('invenio_upgrader') logger.info For longer running upgrade, using info is fine. For shorter upgrade, don't bother about info. For your usage, using the logger is fine, but only the logger.info(). Instead of logger.error you should use raise RuntimeErrorr(msg1, msg2, ..). Same goes for warnings. Use warnings.warn(msg) instead of logger.warn(). This will ensure consistency in that error should stop the migration process, warnings, you are logged and can be continued from. E.g. if you I.e: + logger.error("Failed fixing the record %s" % (str(recid))) Should be raise RuntimeError, or warnings.warn, depending on if you want to stop the migration script or not. I assume you would want to use warnings.warn pre_upgrade(): If you raise the runtime error, you'll stop the entire upgrade process, which I dont' think you want to do. YOu just want to skip it this upgrade right? So instead, just check if more_info is there, set a global varialbe or similar, that do_upgrade will check to see it it shoudl skip the upgrade. In the pre_upgrade, you can issue a warning, saying that it will skip it. post_upgrade(): If you don't use it, you can just remove the method. > > > The questions (about what I don't understand after reading the documentation > .... in inveniocfg_upgrader.py) : > > 1) Modules are supposed to be standalone, not depending too much on imports > from Invenio "unless it is absolutely necessary". > - which imports can be considered necessary ? Upgrade scripts cannot be > ammended in the future (according to the doc), making this work seemingly an > impossible task. What is meant with "cannot be changed in the future", is that if you create a table, and see that you forgot to add a column, then you should create another upgrade if it's already committed to master. If there's a bug in the upgrade, you can naturally fix it. > For instance (example from this upgrade) if I need to import some > configuration variables which have got renamed in one of the commits (in this > example together with moving bibdocfile from websubmit to bibdocfile module). > If we decide to further rename those varaibles, we have to ammend all > upgrade scripts, otherwise it will clearly fail. > Is there a list of safe imports ? Safe imports are currently run_sql and wait_for_user. When say 10 upgrades are committed, we'll have a look to see if we need more (very likely). So again, you can import other mehtods, but if you do, put the import in one of your methods (e.g. do_upgrade) > > 2) How can I mark that a certain commit introduces given changes ? > > Afetr I have installed the commit and created database using invenioconfig > script, intuitively I would expect everything being up to date and not > requiring upgrades. > In reality, the upgrader suggests a series of actions (which cause warnings > because indeed they seem to be already applied). > The dependency system seems to be related not to commits but previously run > scripts. > > Is there any mechanism allowing to mark that a given commit gives Invenio > in the state as after running of a given script ? For example running > bibdocfile model upgrade should not happen if database was created using > newer commit from the repository. > I have not managed to find something like this in the code :( Cheers, Lars > > Piotr
smime.p7s
Description: S/MIME cryptographic signature
