Remove default components and milestones in `trac-admin initenv`

[anonymous]

When initializing a new environment it will be populated with placeholder components and milestones (ie "component#" and "milestone#").

However I think it's rather unlikely that these actual components and milestones will be useful in an actual project. This means that the next step is likely to manually remove these components and milestones and populate the environment with relevant components and milestones.

I suggest that trac-admin initenv instead create an environment without any components or milestones predefined or if required prompt the user for the actual names of the components and milestones.

[Ryan J Ollos]

We can consider adding a --no-default-data option.

[Ryan J Ollos]

r16341 moved the DatabaseManager.insert_into_tables call to the DatabaseManager class via implementation of IEnvironmentSetupParticipant. The motivation was to reduce coupling between the modules.

The change would either need to be reverted, or we would need a way to pass a parameter to environment_created. We could add an argument to IEnvironmentSetupParticipant.environment_created for passing parameters.

PoC changes in [d9b04db04/rjollos.git]. Note that the --no-default-data argument results in skipping insertion of all default data: components, milestones, enums, permissions and reports. We could divide this into "sample data" and "default data" (i.e. default configuration data). Milestones, components and versions are clearly sample data. However, whether enums are sample data is a bit more subjective, and I think that permissions and reports are clearly default data.

If anything is done related to this ticket, I think the aim should be to give more control over how the environment is populated with data, not to extend initenv so that resources like milestones can be added while interactively running initenv. Note that everything request in comment:description can be scripted using trac-admin commands. For example, use the TracAdmin milestone remove / milestone add commands, or milestone rename.

[Ryan J Ollos]

I've experimented a bit, and now have the following proposed changes:

• --no-default-data will yield an empty environment: no reports, milestones, components, permission groups, etc. This gives the user two scripting options: Start with no data and add (e.g. trac-admin milestone add ..), or start with sample data and delete what's unwanted.
• Add method DatabaseManager.insert_default_data, which is called from Environment.create.
• The database version entries in the system tables are always inserted when the database is initialized, set by DatabaseManager.init_db (this is a minor API change).

log:rjollos.git:t12965_initenv_no_default_data.2

Thoughts?

If it looks okay, I'll tidy up the docstrings and add test coverage.

[Ryan J Ollos]

I will add test coverage and proceed to push the changes in absence of any feedback.

Rebased in log:rjollos.git:t12965_initenv_no_default_data.3 —squashed→ [29a8d17a0/rjollos.git]

[Jun Omae]

The changes look good to me.

[Ryan J Ollos]

Thanks for review.

Added tests: [16684974a/rjollos.git].

[Ryan J Ollos]

Committed to trunk in r17162.

[Jun Omae]

After r17162, unit tests with PostgreSQL on my environment. It seems to be unable to reset PostgreSQL database for the unit tests.

$ sudo -u postgres psql -U postgres -tc 'SELECT version()'
 PostgreSQL 9.1.20 on x86_64-unknown-linux-gnu, compiled by gcc (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3, 64-bit

$ make db=postgres clean Trac.egg-info trac/db/tests/__init__.py
find . -name \*.py[co] -exec rm {} \;
coverage erase
rm -fr build/doc

Python: /home/jun66j5/venv/py27/bin/python

  Package        Version
  -------------------------------------------------------
  Python       : 2.7.12 (default, Nov 12 2018, 14:36:49)
               : [GCC 5.4.0 20160609]
  Setuptools   : 20.7.0
  Pip          : 8.1.1
  Wheel        : 0.29.0
  Jinja2       : 2.10.1
  Babel        : 2.7.0
  sqlite3      : 2.6.0 (3.11.0)
  PySqlite     : 2.8.3 (3.11.0)
  PyMySQL      : 0.9.3
  Psycopg2     : 2.8.2 (dt dec pq3 ext lo64)
  SVN bindings : 1.9.3 (r1718519)
  Mercurial    : 4.9.1
  Pygments     : 2.3.1
  Textile      : 3.0.3
  Pytz         : 2019.2
  Docutils     : 0.15.2
  Twill        : 0.9
  LXML         : 4.3.3
  coverage     : 4.5.3
  figleaf      : not installed

Variables:
  PATH=/home/jun66j5/venv/py27/bin:/home/jun66j5/bin:/home/jun66j5/.gem/ruby/2.0.0/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games
  PYTHONPATH=.
  TRAC_TEST_DB_URI=postgres://tracuser:password@localhost/trac?schema=trac_4242424242
  server-options= -p 3000 -a '*,/home/jun66j5/src/trac-htdigest.txt,auth'  -e

External dependencies:
  Git version: git version 2.23.0
  Subversion version: 1.9.3

python  setup.py egg_info
running egg_info
writing requirements to Trac.egg-info/requires.txt
writing Trac.egg-info/PKG-INFO
writing top-level names to Trac.egg-info/top_level.txt
writing dependency_links to Trac.egg-info/dependency_links.txt
writing entry points to Trac.egg-info/entry_points.txt
reading manifest file 'Trac.egg-info/SOURCES.txt'
writing manifest file 'Trac.egg-info/SOURCES.txt'
python  -m unittest  trac.db.tests.__init__.test_suite
...............EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE.....
======================================================================
ERROR: test_insert_empty (trac.db.tests.api.StringsTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "trac/db/tests/api.py", line 112, in setUp
    self.env = EnvironmentStub()
  File "trac/core.py", line 141, in __call__
    self.__init__(*args, **kwargs)
  File "trac/test.py", line 387, in __init__
    self.reset_db(default_data)
  File "trac/test.py", line 438, in reset_db
    dbm.init_db()
  File "trac/db/api.py", line 287, in init_db
    self.set_database_version(version, 'initial_database_version')
  File "trac/db/api.py", line 490, in set_database_version
    current_database_version = self.get_database_version(name)
  File "trac/db/api.py", line 438, in get_database_version
    """.format(db.quote('system')), (name,)):
  File "trac/db/util.py", line 129, in execute
    cursor.execute(query, params if params is not None else [])
  File "trac/db/util.py", line 73, in execute
    return self.cursor.execute(sql_escape_percent(sql), args)
UndefinedTable: relation "system" does not exist
LINE 2:                     SELECT value FROM "system" WHERE name='i...
                                              ^


======================================================================
ERROR: test_insert_markup (trac.db.tests.api.StringsTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "trac/db/tests/api.py", line 112, in setUp
    self.env = EnvironmentStub()
  File "trac/core.py", line 141, in __call__
    self.__init__(*args, **kwargs)
  File "trac/test.py", line 387, in __init__
    self.reset_db(default_data)
  File "trac/test.py", line 438, in reset_db
    dbm.init_db()
  File "trac/db/api.py", line 285, in init_db
    connector.init_db(**args)
  File "trac/db/postgres_backend.py", line 174, in init_db
    cursor.execute('CREATE SCHEMA ' + _quote(cnx.schema))
  File "trac/db/util.py", line 74, in execute
    return self.cursor.execute(sql)
DuplicateSchema: schema "trac_4242424242" already exists

(DuplicateSchema error 69 times)

----------------------------------------------------------------------
Ran 71 tests in 0.882s

FAILED (errors=51)
Makefile:66: recipe for target 'trac/db/tests/__init__.py' failed
make: *** [trac/db/tests/__init__.py] Error 1

[Jun Omae]

Replying to Jun Omae:

After r17162, unit tests with PostgreSQL on my environment. It seems to be unable to reset PostgreSQL database for the unit tests.

Before r17162, DatabaseManager.init_db() creates new connection, execute DDLs and closes the connection. After r17162, the method use a connection in pool to set version to system table after executing DDLs.

Then, a connection without schema is wrongly reused and relation "system" does not exist is raised.

416     def reset_db(self, default_data=None):
417         """Remove all data from Trac tables, keeping the tables themselves.
418
419         :param default_data: after clean-up, initialize with default data
420         :return: True upon success
421         """
422         from trac import db_default
423         tables = []
424         dbm = DatabaseManager(self)
425         try:
426             db_version = dbm.get_database_version()  # <== connection without schema is pooled
427         except (TracError, self.db_exc.DatabaseError):
428             pass
429         else:
430             if db_version == db_default.db_version:
431                 # same version, simply clear the tables (faster)
432                 tables = dbm.reset_tables()
433             else:
434                 # different version or version unknown, drop the tables
435                 self.destroy_db()
436
437         if not tables:
438             dbm.init_db()              # <== reuse connection without schema but schema exists
439             # Make sure the next db_query()/db_transaction() will create
440             # a new connection aware of the new data model - see #8518.
441             if self.dburi != 'sqlite::memory:':
442                 dbm.shutdown()

Ad hoc patch is here. Ideally, I think DatabaseManager.init_db() shouldn't use a connection in pool.

trac/test.py

@@ -435 +435 @@
                 self.destroy_db()
 
         if not tables:
-            dbm.init_db()
             # Make sure the next db_query()/db_transaction() will create
             # a new connection aware of the new data model - see #8518.
             if self.dburi != 'sqlite::memory:':
                 dbm.shutdown()
+            dbm.init_db()
 
         if default_data:
             dbm.insert_default_data()

[Ryan J Ollos]

I've not encountered the errors running Postgres, but my db URI does not have a schema (TRAC_TEST_DB_URI=postgres://tracuser:tracpassword@localhost:5432/trac).

Tests also pass with your proposed patch.

[Jun Omae]

Confirmed with PostgreSQL 9.1 through 11 on circleci at ​py27-pgsql9.1, ​py27-pgsql9.2, …, ​py27-pgsql11. Reproduced the issue only with PostgreSQL 9.1, however tests pass with PostgreSQL 9.2 through 11.