Repository Management #3196

oliverguenther · 2015-06-29T15:46:05Z

This PR begins the push towards management of repository lifetime
from within OpenProject.

What this currently provides is the following:

A mixin ManageableRepository, included by Repository::{Git, Subversion} to provide an interface to repo management
Two services and delayed_job workers for creating (CreateManagedRepositoryService) and
deleting (DeleteRepositoryService)
Customized rendering of subforms in the repository settings for
arbitrary SCM vendors.
Provide sane ground to work on ( cf. state of [now closed] PR Repository Management - Refactoring and Preparation #3149 )
Rework SCM settings form ({scm}_tags is currently HTML generated
from within a Helper.)
Ease integration of arbitrary SCM vendors
Display meaningful error / warning when user looks at bare
repository (currently outputs 500, as Git HEAD / svn revision is not
available)
I18n
Specs
- Fix legacy specs
Decide what to do with Filesystem adapter ~~Will be removed in subsequent PR~~ Merged into this PR due to the errors that would occur when using existing Filesystem repositories on these features.

Review remarks

Manged repositories are not enabled by default. The following section in configuration.yml currently controls the management.

  scm:
    Git:
      # Use command below to override the default git command taken from path.
      # client_command: /usr/local/bin/git
      manages: /tmp/git
    Subversion:
      manages: /tmp/svn
      # Use command below to override the default svn command taken from path.
      # client_command: /usr/local/bin/svn

Creation / Deletion of managed repositories is asynchronous. Don't forget to start a delayed_job worker.
Rubocop complains about several complexity issues in the (git) adapter. I deem this refactoring to be necessary, but out of scope for this PR. I suggest we instead move to Libgit2/rugged for reading a Git repository instead, which would also solve remote repository retrieval
Hound still has some remaining remarks on other files which are either outdated or which I didn't really touch during this PR.
Use the following bookmarklet to toggle outdated hounds:

javascript:(function()%7B%24('.outdated-diff-comment-container').has('img%5Balt%3D"%40houndci"%5D').toggle()%7D)()

Open Questions:

Configuration: Currently replaces the existing options in
configuration.yml.
Implementation specifics:
- Sub-projects repositories reside in a hierarchy below their parents
- How should the delayed_job react upon failure? (esp. creation of
  repositories)

Relevant work packages:

TeatroIO · 2015-06-29T15:46:06Z

I've prepared a stage to preview changes. Open stage or view logs.

houndci-bot · 2015-06-29T15:46:13Z

lib/open_project/scm/manageable_repository.rb

+      # Reads from configuration whether repositories of this kind
+      # may be managed from OpenProject.
+      def manageable?
+        ! managed_root.nil?


Do not leave space between ! and its argument.

houndci-bot · 2015-06-29T15:46:16Z

lib/open_project/scm/adapters/git.rb

+          revisions
+        end
+
+        def build_revision_args(path, identifier_from, identifier_to, options)


Assignment Branch Condition size for build_revision_args is too high. [23.85/15]

houndci-bot · 2015-06-29T15:46:18Z

lib/open_project/scm/adapters/git.rb

+          )
+        end
+
+        def revisions(path, identifier_from, identifier_to, options = {})


Assignment Branch Condition size for revisions is too high. [63.58/15]

houndci-bot · 2015-06-29T15:49:28Z

app/helpers/repositories_helper.rb

-               method: :get,
-               with: "Form.serialize(this.form)")
+               data: { remote: true,
+                       url: url_for(controller: "repositories",


Prefer single-quoted strings when you don't need string interpolation or special symbols.

houndci-bot · 2015-07-23T07:51:05Z

spec/lib/open_project/scm/adapters/subversion_adapter_spec.rb

+  end
+
+  describe 'repository with authorization' do
+    let(:adapter)  { OpenProject::Scm::Adapters::Subversion.new url, root_url, login, password}


Unnecessary spacing detected.
Space missing inside }.

This commit adds / extends the following specs: RepositoriesController: Ensure empty repository page is shown for empty repositories adapters/subversion: Test correct use of --login, --password arguments

ulferts · 2015-07-23T10:50:11Z

db/migrate/20150629075221_add_scm_type_to_repositories.rb

+# Adds the scm_type column to the repositories table and updates all previous
+# repository to correspond to these types.
+#
+# As until now, OP only supported local Git repositories and local or remote Subversion


@oliverguenther why do you differentiate between 'local' and 'existing'. I have scanned the code but couldn't find the explanation. Can you enlighten me?

The reason was to differentiate between existing, but local-only repositories (Git does not allow remote repositories currently, and will not after this PR), and Subversion's URL-based existing repositories (which doesn't care whether the repository is accessed on the filesystem, or on a remote server).

I could have named the scm_type for Git existing as well, but the reason I chose not to was to make some later wishlist upgrade that allows remote Git repositories to change that type back to existing for consistency.

In the end, all that this type controls is the partial that is rendered from /repositories/settings//. It only does more when it is :managed.

ulferts · 2015-07-23T11:04:12Z

When deleting a repository, we only check if it was created as managed, and allow to remove it completely (including directory on filesystem)

I'd go with this option.

Note from @oliverguenther: Deletion of managed repositories, even when configuration no longers permits this is possible with oliverguenther@ca04363

@ulferts

This commit (temporarily) removes the delayed creation and deletion of managed repositories for the sake of improvded error handling. After consultation with @ulferts, we suggest to improve the `Delayed::Job` pipeline to create means for identifying jobs (referenced ID) and to create a unified service to manage database AND filesystem access in a transactional manner. Also provides the following corrections: - Improve migration speed - Delete parent projects only by their identifier, not the managed root

Previously, repositories could not be deleted on disk when the `manages:` configuration entry was removed in the meantime. This commit allows repositories to be deleted in that case because we no longer depend on accessing the managed_root (path for `manages:`).

ulferts · 2015-07-23T12:27:45Z

app/services/scm/create_managed_repository_service.rb

+      # Instead, this will be refactored into a single service wrapper for
+      # creating and deleting repositories, which provides transactional DB access
+      # as well as filesystem access.
+      Scm::CreateRepositoryJob.new(repository).perform


Now that this is synchronous, I think this should actually somehow return the errors e.g. not permitted.

The Job does not catch any errors and thus, will raise when a directory could not be created or a permission error occurred. It will escalate to the repository itself. I will extend it however to catch intermittent exceptions and set localized_rejected_reason instead.

oliverguenther · 2015-07-23T13:23:21Z

I've now added error handling for OS errors (a special case for permission errors as this is likely to happen, and a handler for all Errno ( == SystemCallError) exceptions.

I also found an issue with how the managed path is accessed for created repositories, which is why this commit took a bit more time. Now, all views and services use root_url consistently for the managed path, and url for the managed URL, as managed_repository_{url,path} may raise an error when the managed configuration is no longer accessible.

This commit: - adds error messages for filesystem errors - Forces services and views to use repository.root_url instead of managed_path, which may not be available when config is missing

oliverguenther · 2015-07-23T14:20:25Z

I've detected one more issue when updating the settings, the method was invalid and apparently, a spec was missing for that. Added both now.

oliverguenther · 2015-07-24T08:05:55Z

@ulferts I've skimmed over all your remarks again and If I'm not mistaken, we have covered the open points. I will address asynchronous error handling in a separate WP / PR.

ulferts · 2015-07-24T11:48:44Z

Sorry, found one more issue:

Why should I, as a user, need a save button here? I cannot alter the information in the form anyway.

Note from @oliverguenther:
Button is hidden for managed repositories in oliverguenther@8af9af2

oliverguenther · 2015-07-24T11:57:57Z

For the current set of attributes, you're right. I suggest to take it out now, but we may need some logic to ask the repository whether repositories (of some unknown third-party vendor) may have attributes that can be updated even when they are managed.

oliverguenther · 2015-07-24T15:18:06Z

The checkout plugin btw. has to override this latest change and display the button. I've fixed this by letting the helper decide whether to display the button for the given repository and patch that method instead.

oliverguenther · 2015-07-24T15:20:58Z

Relevant work package: https://community.openproject.org/work_packages/21037

This commit removes the settings `save` button for managed repositories. Important: When adjustable settings for any vendor should be added, this button must be shown depending on some individual vendor logic.

This commit provides the functionality required by the checkout plugin (and possibly others) to change certain attributes and extend views in the repositories settings scope.

…ement Repository Management

oliverguenther added the work in progress label Jun 29, 2015

houndci-bot reviewed Jun 29, 2015
View reviewed changes

oliverguenther added this to the 4.3 milestone Jun 29, 2015

houndci-bot reviewed Jun 29, 2015
View reviewed changes

oliverguenther self-assigned this Jun 29, 2015

houndci-bot reviewed Jun 29, 2015
View reviewed changes

oliverguenther force-pushed the feature/repository_management branch from 9ac429e to a8b5fec Compare June 29, 2015 15:49

houndci-bot reviewed Jun 29, 2015
View reviewed changes

oliverguenther force-pushed the feature/repository_management branch 2 times, most recently from e002848 to 6ac12a2 Compare June 29, 2015 15:58

houndci-bot reviewed Jul 23, 2015
View reviewed changes

Extended specs

9336cff

This commit adds / extends the following specs: RepositoriesController: Ensure empty repository page is shown for empty repositories adapters/subversion: Test correct use of --login, --password arguments

oliverguenther force-pushed the feature/repository_management branch from bd2cb70 to 9336cff Compare July 23, 2015 09:48

ulferts reviewed Jul 23, 2015
View reviewed changes

oliverguenther added 2 commits July 23, 2015 14:13

ulferts reviewed Jul 23, 2015
View reviewed changes

oliverguenther added 3 commits July 23, 2015 16:19

Rework managed repositories error handling

edf713f

This commit: - adds error messages for filesystem errors - Forces services and views to use repository.root_url instead of managed_path, which may not be available when config is missing

Fix typo in attributes-group.

5cb81d6

Fix invalid method used in :update

ad0632a

oliverguenther force-pushed the feature/repository_management branch from 3b69b8a to ad0632a Compare July 23, 2015 14:20

Fix skipped legacy spec due to removed subversion test directory

325ddf6

oliverguenther removed the work in progress label Jul 24, 2015

Remove save button for managed repositories

6763fe8

This commit removes the settings `save` button for managed repositories. Important: When adjustable settings for any vendor should be added, this button must be shown depending on some individual vendor logic.

oliverguenther force-pushed the feature/repository_management branch from 923430d to baf48fe Compare July 24, 2015 18:26

Provide functionality to override settings and views for checkout

aec6593

This commit provides the functionality required by the checkout plugin (and possibly others) to change certain attributes and extend views in the repositories settings scope.

oliverguenther force-pushed the feature/repository_management branch from baf48fe to aec6593 Compare July 24, 2015 19:51

ulferts added a commit that referenced this pull request Jul 27, 2015

Merge pull request #3196 from oliverguenther/feature/repository_manag…

91d68ea

…ement Repository Management

ulferts merged commit 91d68ea into opf:release/4.3 Jul 27, 2015

oliverguenther deleted the feature/repository_management branch October 7, 2015 09:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Repository Management #3196

Repository Management #3196

oliverguenther commented Jun 29, 2015

TeatroIO commented Jun 29, 2015

houndci-bot Jun 29, 2015

houndci-bot Jun 29, 2015

houndci-bot Jun 29, 2015

houndci-bot Jun 29, 2015

houndci-bot Jul 23, 2015

ulferts Jul 23, 2015

oliverguenther Jul 23, 2015

ulferts commented Jul 23, 2015

ulferts Jul 23, 2015

oliverguenther Jul 23, 2015

ulferts Jul 23, 2015

oliverguenther commented Jul 23, 2015

oliverguenther commented Jul 23, 2015

oliverguenther commented Jul 24, 2015

ulferts commented Jul 24, 2015

oliverguenther commented Jul 24, 2015

oliverguenther commented Jul 24, 2015

oliverguenther commented Jul 24, 2015

Repository Management #3196

Repository Management #3196

Conversation

oliverguenther commented Jun 29, 2015

Review remarks

Open Questions:

TeatroIO commented Jun 29, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ulferts commented Jul 23, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliverguenther commented Jul 23, 2015

oliverguenther commented Jul 23, 2015

oliverguenther commented Jul 24, 2015

ulferts commented Jul 24, 2015

oliverguenther commented Jul 24, 2015

oliverguenther commented Jul 24, 2015

oliverguenther commented Jul 24, 2015