Fabrice, I think your branch of qserv_testdata needs some minor cleanup even before you send it for review:

• there are more than 20 commits, I think some of them can be squashed without losing anything to reduce the amount of code that reviewers need to check:
  • e.g. bug fix in 18a8adc05ff128d90 is a good candidate for squashing, same is probably true for ff190b3e552d14 and few others
  • some files are added and then removed later (.gitignore, fetch-data.sh), I'm not sure they even need to be in a history

Could you try to clean it up a bit and probably rebase it to master at the same time?

Hi Andy,

Ok, Jacek helped me to squash them in three larger and consistent commits.

Cheers,

Fabrice

Here are my review comment (after squashing, thanks!):

• in DM-1720 I added dirTable parameter to common.cfg files, here you also added it to case01/data/Object.cfg and case02/data/Object.cfg which duplicates common.cfg. dirTable is per-database parameter and it should be in common.cfg
• same applies to case04, dirTable should be in common.cfg, not individual Table.cfg
• do you know why is dirColName needed in Object.cfg, my impression was that it's only useful for non-director tables.

dataConfig.py

• line 33: urljoin import is not used
• line 50: we should use doxygen tags in python docstrings (https://confluence.lsstcorp.org/display/LDMDG/Documentation+Standards#DocumentationStandards-Python) so parameter tag should be @param, short description of what parameter is is also useful Same for :return below
• lines 86-87:

  v = self['tables'].get('views')

  return v if v else []

  it could be written as

  return self['tables'].get('views', [])

  which I think does the same but is somewhat shorter

dataCustomizer.py:

• unused import urllib, sys
• line 63: run method has no parameters but docstring shows a bunch of them

unittest/testDataConfig.py:

• Unused commons imported from lsst.qserv.admin
• line 55: Instance of 'TestDataConfig' has no 'logger' member
• line 78: Module 'lsst.qserv.admin.logger' has no 'get_default_log_conf' member

unittest/testDataCustomizer.py

• line 74: Module 'lsst.qserv.admin.logger' has no 'get_default_log_conf' member

Documentation (in qserv package) feels a bit terse, people who are not very familiar with how integration test script works may get confused. Maybe worth to elaborate a bit, especially on where large data comes from and how much disk space needed for that?

Fabrice, I am looking at DM-1571, and my first thought was: "hmm, I really want to know how to do that." Say I want to re-setup the system for ipac/sui tests, would I be able to do that based on documentation? I second Andy's comment, please update the documentation as necessary so that it is easy to find and follow. Thanks!

Hi Andy

• {dirTables}}: OK, I think git may have done this in my back
• as far as I can remember, dirColName seems to be required in CSS for director table also. Qserv fails if not provided.
• urljoin: OK
• docstring: OK
• lines 86-87:

  v = self['tables'].get('views')

  return v if v else []

  I've previously written the code you propose but the issue is that if "views" parameter is defined with empty value in yaml, v will be equal to "", and the second form will return "" instead of []. The current version handle this side-effect.

• dataCustomizer.py: OK
• unittest/testDataConfig.py: OK
• unittest/testDataCustomizer.py: OK

Jacek Becla, I've also added some additional informations about data, as proposed by Andy, see: https://lsst-web.ncsa.illinois.edu/~fjammes/qserv-doc/2015_01/admin/gb-sized-integration-test.html

Thanks Andy.