Move the repository_sync_per_request configuration option to the repositories section

[Ryan J Ollos]

As described in comment:1:ticket:11703, move the [trac] repository_sync_per_request configuration option to the [repositories] section:

[repositories]
<name>.sync_per_request = true/false

and similarly add an attribute for repositories defined through DbRepostioryProvider. Also, allow the option to be configurable from the repositories web admin page. This might help with configuration issues like the one reported in #11767.

[Ryan J Ollos]

Proposed changes in log:rjollos.git:t11776.1. TracRepositoryAdmin page modified to document these changes in 1.1/TracRepositoryAdmin@2.

[anonymous]

Looking good. +1

[Ryan J Ollos]

Changes for #11703 will need to be committed before these changes. Committing these changes first leads to test failures since there's no way to define explicit synchronization for the repository defined in [trac] repository_dir, and some tests in trac/versioncontrol/tests/functional.py depend on explicit synchronization.

[Jun Omae]

I just confirmed rjollos.git@t11776.1.

1. Even if sync_per_request is 0, sync is executed by each request.
2. Synchronized '(default)' repository … is not logged even if sync is successful.
3. If Sync on every request is Not recommended, I think the option should be disabled for new repository by default. Adding a large repository from repository admin panel, a user would have a long wait for next request.

trac/versioncontrol/api.py

@@ -221 +221 @@
                 "INSERT INTO repository (id, name, value) VALUES (%s, %s, %s)",
                 [(id, 'dir', dir),
                  (id, 'type', type_ or ''),
-                 (id, 'sync_per_request', '1')])
+                 (id, 'sync_per_request', '0')])
         rm.reload_repositories()
 
     def add_alias(self, reponame, target):
@@ -336 +336 @@
         from trac.web.chrome import Chrome, add_warning
         if handler is not Chrome(self.env):
             for repo_info in self.get_all_repositories().values():
-                if not repo_info.get('sync_per_request'):
+                if not as_bool(repo_info.get('sync_per_request')):
                     continue
                 start = time.time()
                 repo_name = repo_info['name'] or '(default)'
                 try:
                     repo = self.get_repository(repo_info['name'])
                     repo.sync()
-                    continue
                 except TracError as e:
                     add_warning(req,
                         _("Can't synchronize with repository \"%(name)s\" "

[Jun Omae]

4. We should use i18n:msg for span of new hint.

trac/versioncontrol/templates/admin_repositories.html

@@ -86 +86 @@
                           disabled="${not info.editable or None}" />
               Sync on every request
             </label>
-            <span class="hint">Not recommended. See <a href="${href.wiki('TracRepositoryAdmin') + '#ExplicitSync'}">explicit synchronization</a> for more info.</span>
+            <span class="hint" i18n:msg="">Not recommended. See <a href="${href.wiki('TracRepositoryAdmin') + '#ExplicitSync'}">explicit synchronization</a> for more info.</span>
           </div>
           <div class="field">
             <label><input type="checkbox" name="hidden" value="1" checked="${info.hidden or None}"

[Ryan J Ollos]

Thanks for the review and patches. Committed to trunk in [13393:13394].

[Jun Omae]

Sorry. I hadn't run functional tests. After [13394], trac.versioncontrol.tests.functional fail. The functional tests expect testenv has default repository's sync_per_request is enabled.

trac/tests/functional/svntestenv.py

@@ -44 +44 @@
         repo as well."""
         pass
 
+    def post_create(self, env):
+        """Hook for modifying the environment after creation."""
+        self._tracadmin('config', 'set', 'repositories', '.sync_per_request',
+                        '1')
+
     def repo_url(self):
         """Returns the url of the Subversion repository for this test
         environment.

[Ryan J Ollos]

I thought I ran functional tests, but I can reproduce the issue locally so I must not have ran them. I guess functional tests fail on automated builds only for Python 2.7 because we don't have SVN bindings for Python 2.6 and therefore the failing tests are skipped? The tests are clearly run on Python 2.7 with system site packages, but make status is reporting SVN bindings : not installed.

Thanks for the patch. Fixed in [13404].

[Jun Omae]

Yes. Travis CI VM is based on Ubuntu 12.04 LTS. The python-subversion is available only on python 2.7 with system site packages. Also, Subversion on Ubuntu 12.04 is 1.6.17. But svn.core.SVN_VERSION is available after Subversion 1.7.

attachment:fix-svn-version-in-make-status.diff​

[Jun Omae]

attachment:fix-svn-version-in-make-status.diff​

When Subversion is not installed with the patch, an ImportError would be raised…. Revised patch is applied in [13406-13408]. make status on autobuilds is reporting SVN bindings : 1.6.17 (r1128011), now.

[Christian Boos]

Replying to jomae:

attachment:fix-svn-version-in-make-status.diff​

make status on autobuilds is reporting SVN bindings : 1.6.17 (r1128011), now.

Thanks for fixing this! I indeed didn't test with 1.6…

[Peter Suter]

Replying to rjollos:

Committed to trunk in [13393:13394].

This seems to conflict with [13338]: #11885

[Ryan J Ollos]

repository_sync_per_request option is not being removed if the option is empty. Proposed fix in: log:rjollos.git:remove_repos_sync_per_request.

[Jun Omae]

I noticed db32.py has a issue when GitwebProjectsRepositoryProvider (IRepositoryProvider components) and the repository from the component is listed in repository_sync_per_request option.

So, after repository_sync_per_request option is removed, no easy way to use sync_per_request feature for the repositories from IRepositoryProvider components except RepositoryManager (trac.ini) and DbRepositoryProvider….

[Ryan J Ollos]

I'll review GitwebProjectsRepositoryProvider and give some thought to have we can handle the issue.

[Ryan J Ollos]

comment:16 changes committed in [14692].

[Ryan J Ollos]

Replying to Jun Omae:

I noticed db32.py has a issue when GitwebProjectsRepositoryProvider (IRepositoryProvider components) and the repository from the component is listed in repository_sync_per_request option.

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 =

[Ryan J Ollos]

Since this ticket was resolved in milestone:1.1.3, I've created #12480 to address comment:17 in milestone:1.2.

[Ryan J Ollos]

Sync per request option on admin repository detail page was incorrectly shown for an Alias. Fixed issue in r16107, merged to trunk in r16108.

[Ryan J Ollos]

Documented in r16250, merged to trunk in r16251.