Edgewall Software
Modify

Opened 5 years ago

Closed 4 years ago

Last modified 3 years ago

#11587 closed enhancement (fixed)

Implement ISystemInfoProvider in git connector and database connection classes

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.1.2
Component: general Version:
Severity: normal Keywords:
Cc:
Release Notes:

The GitConnector and SubversionConnector are required components when a repository of their type is configured. When required, the Component cannot be disabled from the plugin admin page and its version is displayed on the About and Error pages. With these changes the behavior of the repository connectors matches that of database connectors.

API Changes:

The db, web server and git version info are not directly appended to Environment.systeminfo. The version is instead provided through the ISystemInfoProvider interface. Environment.systeminfo might be made a private attribute in a future release of Trac.

Description

It was shown in comment:13:ticket:11565 that there are some places in the code in which the Environment.systeminfo list is directly appended to. It might be better if every component providing systeminfo data did so through ISystemInfoProvider.

The database version info was added in [4420]. The ISystemInfoProvider interface was added in [9242]. In comment:6:ticket:8908 it was noted why the database connector classes still append directly to systeminfo. Care will be given to that issue when preparing the patch.

Attachments (0)

Change History (7)

comment:1 Changed 4 years ago by Ryan J Ollos

Owner: set to Ryan J Ollos
Status: newassigned

comment:2 Changed 4 years ago by Ryan J Ollos

Some proposed changes in log:rjollos.git:t11587. The changes don't result in a great simplification, but they do narrow the responsibilities of get_connection and by not appending directly to Environment.systeminfo the consistent pattern of providing info through ISystemInfoProvider is used everywhere.

I'm still considering the change in [6ff3cd7f/rjollos.git]. If the change is accepted then SubversionConnector should be modified to behave in the same way.

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

comment:3 in reply to:  2 ; Changed 4 years ago by Jun Omae

Some proposed changes in log:rjollos.git:t11587.

If MySQLdb is not installed and mysql://... is used, the following NameError is raised.

$ PYTHONPATH=. ~/venv/py26-1.1/bin/python trac/admin/console.py /dev/shm/t11587 initenv t11587 mysql://tracuser:password@localhost/t11587
Creating and Initializing Project
Initenv for '/dev/shm/t11587' failed.
Failed to create environment.
global name 'MySQLdb' is not defined
Traceback (most recent call last):
  File "trac/admin/console.py", line 458, in do_initenv
    options=options)
...
  File "/home/jun66j5/src/tracdev/git/trac/db/mysql_backend.py", line 89, in __init__
    self._mysqldb_version = get_pkginfo(MySQLdb).get('version',
NameError: global name 'MySQLdb' is not defined

We should check has_mysqldb before use of get_pkginfo(MySQLdb)

  • trac/db/mysql_backend.py

    diff --git a/trac/db/mysql_backend.py b/trac/db/mysql_backend.py
    index 7a9aad1..d2439c9 100644
    a b class MySQLConnector(Component):  
    8686
    8787    def __init__(self):
    8888        self._mysql_version = None
    89         self._mysqldb_version = get_pkginfo(MySQLdb).get('version',
     89        self._mysqldb_version = has_mysqldb and \
     90                                get_pkginfo(MySQLdb).get('version',
    9091                                                         MySQLdb.__version__)
    9192        self.error = None
    9293        self.required = False

comment:4 Changed 4 years ago by Ryan J Ollos

Revised changes in log:rjollos.git:t11587.2. SubversionConnector is now a required component when a subversion repository is configured, and Subversion is only shown on the About page if there is a subversion repository configured. This matches the behavior implemented for the GitConnector in the proposed changes, and for the MercurialConnector after the following patch is applied:

  • tracext/hg/backend.py

    diff -r 251cde2ad7fd tracext/hg/backend.py
    a b  
    381381            m = re.match(r'(\d+)\.(\d+)(?:\.(\d+))?', self._version or '')
    382382            if m:
    383383                self._version_info = tuple([int(n or 0) for n in m.groups()])
     384        self.required = False
    384385
    385386    def _setup_ui(self, hgrc_path):
    386387        # Starting with Mercurial 1.3 we can probably do simply:
     
    418419    # ISystemInfoProvider methods
    419420
    420421    def get_system_info(self):
    421         if self._version is not None:
     422        if self.required:
    422423            yield 'Mercurial', self._version
    423424
    424425    # IRepositoryConnector methods
     
    437438            self._setup_ui(self.hgrc)
    438439        repos = MercurialRepository(dir, params, self.log, self)
    439440        repos.version_info = self._version_info
     441        self.required = True
    440442        return repos
    441443
    442444    # IWikiSyntaxProvider methods

comment:5 in reply to:  3 Changed 4 years ago by Ryan J Ollos

Replying to jomae:

We should check has_mysqldb before use of get_pkginfo(MySQLdb)

I added similar checks in PostgreSQLConnector and SQLiteConnector constructors. I've experimented with some test cases for ImportError of database bindings [b93c3cec/rjollos.git], but that's for future consideration in utilizing mock.

Trac changes committed to trunk in [12820].

comment:6 Changed 4 years ago by Ryan J Ollos

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

Change to MercurialPlugin committed to 1.0 branch in [635936bdbe09/mercurial-plugin]. An additional minor change in [12823].

comment:7 Changed 3 years ago by Ryan J Ollos

[12820#file5] and [12820#file7] partially reverted in [14293] (#12194).

Last edited 3 years ago by Ryan J Ollos (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.
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.