Edgewall Software
Modify

Opened 6 years ago

Closed 5 years ago

Last modified 18 months ago

#12965 closed enhancement (fixed)

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

Reported by: anonymous Owned by: Ryan J Ollos
Priority: normal Milestone: 1.5.1
Component: admin/console Version:
Severity: normal Keywords:
Cc: Branch:
Release Notes:

Added option --no-default-data to trac-admin initenv, for creating an empty environment without any reports, permission groups or sample data (components, milestones, enums).

API Changes:

DatabaseManager.init_db sets the database_version and initial_database_version in the system table.

Internal Changes:

Description

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.

Attachments (0)

Change History (11)

comment:1 by Ryan J Ollos, 6 years ago

Milestone: next-major-releases

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

comment:2 by Ryan J Ollos, 6 years ago

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.

comment:3 by Ryan J Ollos, 5 years ago

Milestone: next-major-releases1.5.1
Owner: set to Ryan J Ollos
Release Notes: modified (diff)
Status: newassigned

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.

comment:4 by Ryan J Ollos, 5 years ago

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]

comment:5 by Jun Omae, 5 years ago

The changes look good to me.

comment:6 by Ryan J Ollos, 5 years ago

Thanks for review.

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

comment:7 by Ryan J Ollos, 5 years ago

API Changes: modified (diff)
Resolution: fixed
Status: assignedclosed

Committed to trunk in r17162.

comment:8 by Jun Omae, 5 years ago

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

in reply to:  8 comment:9 by Jun Omae, 5 years ago

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

    diff --git a/trac/test.py b/trac/test.py
    index b8db30327..691606743 100755
    a b class EnvironmentStub(Environment):  
    435435                self.destroy_db()
    436436
    437437        if not tables:
    438             dbm.init_db()
    439438            # Make sure the next db_query()/db_transaction() will create
    440439            # a new connection aware of the new data model - see #8518.
    441440            if self.dburi != 'sqlite::memory:':
    442441                dbm.shutdown()
     442            dbm.init_db()
    443443
    444444        if default_data:
    445445            dbm.insert_default_data()

comment:10 by Ryan J Ollos, 5 years ago

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.

Last edited 5 years ago by Ryan J Ollos (previous) (diff)

comment:11 by Jun Omae, 5 years ago

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.

Last edited 5 years ago by Jun Omae (previous) (diff)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.