Edgewall Software
Modify

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#12480 closed defect (fixed)

Restore repository_sync_per_request for GitwebProjectsRepositoryProvider

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.2
Component: plugin/git Version:
Severity: normal Keywords: gitweb
Cc: Branch:
Release Notes:

Gitweb repository configuration is moved from the [git] section to the [gitweb-repositories] section.

API Changes:
Internal Changes:

Description

After changes in #11776, there is no way to specify repository_sync_per_request for GitwebProjectsRepositoryProvider.

Since GitwebProjectsRepositoryProvider defines several attributes for specifying repositories, it should probably also have a sync_per_request ListOption. It might also make sense to move the options to a separate section, as was done with the [repositories] section for the default repository provider.

[gitweb-repositories]
projects_base = 
projects_list = 
projects_url = 
sync_per_request =

Attachments (0)

Change History (10)

comment:1 by Ryan J Ollos, 5 years ago

Owner: set to Ryan J Ollos
Status: newassigned

comment:2 by Ryan J Ollos, 5 years ago

Release Notes: modified (diff)

Proposed changes in log:rjollos.git:t12480_gitweb.

comment:3 by Ryan J Ollos, 5 years ago

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

Committed to trunk in [14873:14874].

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

comment:4 by Ryan J Ollos, 5 years ago

The upgrade step fails when tracopt.versioncontrol.git.git_fs.GitwebProjectsRepositoryProvider is not enabled. I'll add a test and fix shortly.

comment:5 by Ryan J Ollos, 5 years ago

I added a method to allow components to be re-enabled in a test case, for use in [91874a31/rjollos.git]. Does it make sense, or is there a better way?: [bb235d88/rjollos.git]. I'm considering applying the change to 1.0-stable.

comment:6 by Jun Omae, 5 years ago

The default of [trac] repository_sync_per_request option is (default). It needed to set explicitly an empty value to the option if don't want repository sync per request.

Therefore, we should use config.getlist(..., ..., '(default)') rather than config.getlist(...) or ['(default)'].

-    repos_sync_per_request = \
-        env.config.getlist('trac', 'repository_sync_per_request') \
-        or ['(default)']
+    repos_sync_per_request = env.config.getlist(
+        'trac', 'repository_sync_per_request', '(default)')

Also, we should add unit tests for repository_sync_per_request with repository table.

+    def test_repository_sync_per_request_default_value_with_db(self):
+        """The default repository sync_per_request attribute is set to true
+        when repository_sync_per_request is not set in trac.ini.
+        """
+        self.env.config.remove('trac', 'repository_sync_per_request')
+        # directly insert repository records instead of DbRepositoryProvider
+        # to avoid a TracError "The repository type 'svn' is not supported"
+        with self.env.db_transaction as db:
+            db.executemany("""INSERT INTO repository (id,name,value)
+                              VALUES (%s,%s,%s)""",
+                           [(1, 'name', ''),
+                            (1, 'dir', '/var/svn'),
+                            (1, 'type', 'svn'),
+                            (2, 'name', 'git'),
+                            (2, 'dir', '/var/git'),
+                            (2, 'type', 'git')])
+
+        db32.do_upgrade(self.env, VERSION, None)
+
+        repos = RepositoryManager(self.env).get_all_repositories()
+        self.assertIn('', repos)
+        self.assertTrue(repos['']['sync_per_request'])
+        self.assertEqual('1', self.env.db_query("""
+            SELECT value FROM repository
+            WHERE id=1 AND name='sync_per_request'""")[0][0])
+        self.assertIn('git', repos)
+        self.assertFalse(repos['git']['sync_per_request'])
+        self.assertEqual('0', self.env.db_query("""
+            SELECT value FROM repository
+            WHERE id=2 AND name='sync_per_request'""")[0][0])
+

in reply to:  5 comment:7 by Ryan J Ollos, 5 years ago

Replying to Ryan J Ollos:

I added a method to allow components to be re-enabled in a test case, for use in [91874a31/rjollos.git]. Does it make sense, or is there a better way?: [bb235d88/rjollos.git]. I'm considering applying the change to 1.0-stable.

This change was committed to 1.0-stable in r14976, merged to 1.2-stable in r14977, merged to trunk in r14978. I'll prepare additional changes with comment:6 suggestions.

comment:8 by Ryan J Ollos, 5 years ago

Committed to 1.2-stable in r14998, merged to trunk in r14999. Thanks for the fix and additional test.

comment:9 by Ryan J Ollos, 5 years ago

Tests were passing locally for me after adding import from tracopt.versioncontrol.svn.svn_fs import SubversionConnector, to allow calling DbRepositoryProvider.add_repository, but fail on Travis CI. I guess that is due to no SVN bindings.

I will probably commit changes with the SQL insert from comment:6.

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

comment:10 by Ryan J Ollos, 5 years ago

Revisions discussed in comment:9 committed to 1.2-stable in r15002,r15008; merged to trunk in r15003,r15009.

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.