Edgewall Software
Modify

Opened 9 years ago

Closed 9 years ago

Last modified 7 years ago

#11776 closed enhancement (fixed)

Move the repository_sync_per_request configuration option to the repositories section

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.1.3
Component: version control Version:
Severity: normal Keywords:
Cc: Branch:
Release Notes:

The [trac] repository_sync_per_request option was removed and each repository now has a sync_per_request attribute. The default value of sync_per_request is False. An upgrade step handles the migration while preserving the behavior.

The sync_per_request attribute can be set on repositories defined in the database from the Repositories web admin page or using the trac-admin command. For repositories defined in the [repositories] section of trac.ini, the <name>.sync_per_request attribute can be edited.

API Changes:
Internal Changes:

Description (last modified by 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.

Attachments (1)

fix-svn-version-in-make-status.diff (2.0 KB ) - added by Jun Omae 9 years ago.

Download all attachments as: .zip

Change History (24)

comment:1 by Ryan J Ollos, 9 years ago

Description: modified (diff)

comment:2 by Ryan J Ollos, 9 years ago

Status: newassigned

comment:3 by Ryan J Ollos, 9 years ago

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

comment:4 by anonymous, 9 years ago

Looking good. +1

comment:5 by Ryan J Ollos, 9 years ago

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.

comment:6 by Jun Omae, 9 years ago

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

    diff --git a/trac/versioncontrol/api.py b/trac/versioncontrol/api.py
    index 9b4e28e..0b33b6b 100644
    a b class DbRepositoryProvider(Component):  
    221221                "INSERT INTO repository (id, name, value) VALUES (%s, %s, %s)",
    222222                [(id, 'dir', dir),
    223223                 (id, 'type', type_ or ''),
    224                  (id, 'sync_per_request', '1')])
     224                 (id, 'sync_per_request', '0')])
    225225        rm.reload_repositories()
    226226
    227227    def add_alias(self, reponame, target):
    class RepositoryManager(Component):  
    336336        from trac.web.chrome import Chrome, add_warning
    337337        if handler is not Chrome(self.env):
    338338            for repo_info in self.get_all_repositories().values():
    339                 if not repo_info.get('sync_per_request'):
     339                if not as_bool(repo_info.get('sync_per_request')):
    340340                    continue
    341341                start = time.time()
    342342                repo_name = repo_info['name'] or '(default)'
    343343                try:
    344344                    repo = self.get_repository(repo_info['name'])
    345345                    repo.sync()
    346                     continue
    347346                except TracError as e:
    348347                    add_warning(req,
    349348                        _("Can't synchronize with repository \"%(name)s\" "
Last edited 9 years ago by Jun Omae (previous) (diff)

comment:7 by Jun Omae, 9 years ago

  1. We should use i18n:msg for span of new hint.
  • trac/versioncontrol/templates/admin_repositories.html

    diff --git a/trac/versioncontrol/templates/admin_repositories.html b/trac/versioncontrol/templates/admin_repositories.html
    index 8ad06a3..7df1daa 100644
    a b  
    8686                          disabled="${not info.editable or None}" />
    8787              Sync on every request
    8888            </label>
    89             <span class="hint">Not recommended. See <a href="${href.wiki('TracRepositoryAdmin') + '#ExplicitSync'}">explicit synchronization</a> for more info.</span>
     89            <span class="hint" i18n:msg="">Not recommended. See <a href="${href.wiki('TracRepositoryAdmin') + '#ExplicitSync'}">explicit synchronization</a> for more info.</span>
    9090          </div>
    9191          <div class="field">
    9292            <label><input type="checkbox" name="hidden" value="1" checked="${info.hidden or None}"

comment:8 by Ryan J Ollos, 9 years ago

Release Notes: modified (diff)

comment:9 by Ryan J Ollos, 9 years ago

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

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

comment:10 by Jun Omae, 9 years ago

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

    diff --git a/trac/tests/functional/svntestenv.py b/trac/tests/functional/svntestenv.py
    index b4cdab6..1fc935f 100644
    a b class SvnFunctionalTestEnvironment(FunctionalTestEnvironment):  
    4444        repo as well."""
    4545        pass
    4646
     47    def post_create(self, env):
     48        """Hook for modifying the environment after creation."""
     49        self._tracadmin('config', 'set', 'repositories', '.sync_per_request',
     50                        '1')
     51
    4752    def repo_url(self):
    4853        """Returns the url of the Subversion repository for this test
    4954        environment.

comment:11 by Ryan J Ollos, 9 years ago

I thought I had 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].

Version 0, edited 9 years ago by Ryan J Ollos (next)

by Jun Omae, 9 years ago

comment:12 by Jun Omae, 9 years ago

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

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

in reply to:  12 ; comment:13 by Jun Omae, 9 years ago

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.

in reply to:  13 comment:14 by Christian Boos, 9 years ago

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…

in reply to:  9 comment:15 by Peter Suter, 9 years ago

Replying to rjollos:

Committed to trunk in [13393:13394].

This seems to conflict with [13338]: #11885

comment:16 by Ryan J Ollos, 8 years ago

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.

comment:17 by Jun Omae, 8 years ago

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….

comment:18 by Ryan J Ollos, 8 years ago

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

comment:19 by Ryan J Ollos, 8 years ago

comment:16 changes committed in [14692].

in reply to:  17 comment:20 by Ryan J Ollos, 8 years ago

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 =

comment:21 by Ryan J Ollos, 8 years ago

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

comment:22 by Ryan J Ollos, 7 years ago

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

comment:23 by Ryan J Ollos, 7 years ago

Documented in r16250, merged to trunk in r16251.

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.