Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Issue 596: cleanup_empty_projects, disable_publish_for_branches options #599

Closed
wants to merge 5 commits into from

3 participants

@smcv

Normally, if a submit request is accepted and results in cleaning up the last package in a branched project, the branched project is deleted too. This is annoying if the branched project is a long-running
branch (e.g. for a developer in a small team) and has been reconfigured, for instance to turn on publishing in installations where branches do not normally have publishing.

Similarly, OBS currently disables publishing for any new branch in order to avoid using disk space and bandwidth to carry out publishing, but for small private installations, it can make sense to avoid that, and inherit the publish flags from the parent project instead.

smcv added some commits
@smcv smcv cleanup_empty_projects: add new configuration option
Normally, if a submit request is accepted and results in cleaning up
the last package in a branched project, the branched project is
deleted too. This is annoying if the branched project is a long-running
branch (e.g. for a developer in a small team) and has been reconfigured,
for instance to turn on publishing in installations where branches do
not normally have publishing.

Bug: openSUSE#596
a1f3bb2
@smcv smcv disable_publish_for_branches: new configuration option
OBS currently disables publishing for any new branch in order to avoid
using disk space and bandwidth to carry out publishing. For small private
installations, it can make sense to avoid that, and inherit the publish
flags from the parent project instead.

Bug: openSUSE#596
0f508c3
@smcv smcv Split out branch_copy_flags method to placate code complexity check 7968c7f
@smcv

Issue #596.

@coveralls

Coverage Status

Coverage remained the same when pulling 7968c7f on smcv:issue596 into 3b33b5e on openSUSE:master.

@adrianschroeter

I still want test cases before merging :)

Please check test/functional/read_permission_test.rb file. I think the test_setup_default_project comes very near to a template for your switches. Just modify the setting and test that the new behaviour comes.

You can run only this test for example via:

ruby test/functional/read_permission_test.rb --name test_setup_default_project

after the you run "rake test" once. It is also a much faster way to develop the code :)

@smcv

Please check test/functional/read_permission_test.rb file. I think the test_setup_default_project comes very near to a template for your switches.

Thanks, I'll look into that. Please bear with me, I don't think I'd written any Ruby before last week :-)

Are you OK with these two configuration options in principle? Unfortunately, for my "production" use of them I'll need to backport them to a rather old branch of OBS, where the actual implementation is not directly applicable, and the regression tests might well not be either; but I'd like the high-level functionality I backport to be something that is likely to be accepted upstream.

@smcv

Tests added. The Travis CI build is failing, but I think that's unrelated: other people's branches are failing in the same way.

@adrianschroeter

the failures are caused by the broken createrepo of ubuntu distro. fixing it by packaging an own createrepo for ubuntu (travis runs ubuntu worker)

I will have a look at the pull request tomorrow again.

@adrianschroeter

thanks, I have merged this now, so it will be most likely part of OBS 2.6 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 19, 2014
  1. @smcv

    cleanup_empty_projects: add new configuration option

    smcv authored
    Normally, if a submit request is accepted and results in cleaning up
    the last package in a branched project, the branched project is
    deleted too. This is annoying if the branched project is a long-running
    branch (e.g. for a developer in a small team) and has been reconfigured,
    for instance to turn on publishing in installations where branches do
    not normally have publishing.
    
    Bug: openSUSE#596
  2. @smcv

    disable_publish_for_branches: new configuration option

    smcv authored
    OBS currently disables publishing for any new branch in order to avoid
    using disk space and bandwidth to carry out publishing. For small private
    installations, it can make sense to avoid that, and inherit the publish
    flags from the parent project instead.
    
    Bug: openSUSE#596
  3. @smcv
Commits on Feb 20, 2014
  1. @smcv
  2. @smcv
This page is out of date. Refresh to see the latest.
View
18 docs/api/api/configuration.rng
@@ -124,6 +124,24 @@
</choice>
</element>
</optional>
+ <optional>
+ <element name="cleanup_empty_projects">
+ <!-- If the last package in a project is cleaned up by sourceupdate=cleanup, delete the whole project? -->
+ <choice>
+ <value>on</value>
+ <value>off</value>
+ </choice>
+ </element>
+ </optional>
+ <optional>
+ <element name="disable_publish_for_branches">
+ <!-- When a user creates a new project as a side-effect of branching a package, disable publishing that project? The default is "on" to save disk space and bandwidth. -->
+ <choice>
+ <value>on</value>
+ <value>off</value>
+ </choice>
+ </element>
+ </optional>
<!-- webui only stuff -->
<optional>
View
2  src/api/app/models/bs_request_action.rb
@@ -415,7 +415,7 @@ def source_cleanup
def source_cleanup_delete_path
source_project = Project.find_by_name!(self.source_project)
- if source_project.packages.count == 1 or !self.source_package
+ if (source_project.packages.count == 1 and ::Configuration.first.cleanup_empty_projects) or !self.source_package
# remove source project, if this is the only package and not a user's home project
splits = self.source_project.split(':')
View
4 src/api/app/models/configuration.rb
@@ -26,8 +26,10 @@ class Configuration < ActiveRecord::Base
:no_proxy => nil,
:cleanup_after_days => nil,
:theme => CONFIG['theme'],
+ :cleanup_empty_projects => nil,
+ :disable_publish_for_branches => nil,
}
- ON_OFF_OPTIONS = [ :anonymous, :default_access_disabled, :allow_user_to_create_home_project, :disallow_group_creation, :change_password, :hide_private_options, :gravatar, :download_on_demand, :enforce_project_keys ]
+ ON_OFF_OPTIONS = [ :anonymous, :default_access_disabled, :allow_user_to_create_home_project, :disallow_group_creation, :change_password, :hide_private_options, :gravatar, :download_on_demand, :enforce_project_keys, :cleanup_empty_projects, :disable_publish_for_branches ]
class << self
def map_value(key, value)
View
26 src/api/app/models/project.rb
@@ -1070,13 +1070,29 @@ def branch_to_repositories_from(project, pkg_to_enable, extend_names=nil)
end
pkg_to_enable.enable_for_repository(repoName) if pkg_to_enable
end
- # take over flags, but explicit disable publishing by default and enable building. Ommiting also lock or we can not create packages
+
+ self.branch_copy_flags(project)
+ end
+
+ def branch_copy_flags(project)
+ # Copy the flags from the other project, adjusting them appropriately
+ # for this one being a branch of it:
+ #
+ # - enable building
+ # - disable 'publish' to save space and bandwidth
+ # (can be turned off for small installations)
+ # - omit 'lock' or we cannot create packages
+ disable_publish_for_branches = ::Configuration.first.disable_publish_for_branches
project.flags.each do |f|
- unless %w(build publish lock).include?(f.flag)
- self.flags.create(status: f.status, flag: f.flag, architecture: f.architecture, repo: f.repo)
- end
+ next if %w(build lock).include?(f.flag)
+ next if f.flag == 'publish' and disable_publish_for_branches
+
+ self.flags.create(status: f.status, flag: f.flag, architecture: f.architecture, repo: f.repo)
+ end
+
+ if disable_publish_for_branches
+ self.flags.create(:status => 'disable', :flag => 'publish') unless self.flags.find_by_flag_and_status( 'publish', 'disable' )
end
- self.flags.create(:status => 'disable', :flag => 'publish') unless self.flags.find_by_flag_and_status( 'publish', 'disable' )
end
def open_requests_with_project_as_source_or_target
View
9 src/api/db/migrate/20140218174400_cleanup_empty_projects.rb
@@ -0,0 +1,9 @@
+class CleanupEmptyProjects < ActiveRecord::Migration
+ def self.up
+ add_column :configurations, :cleanup_empty_projects, :boolean, :default => true
+ end
+
+ def self.down
+ remove_column :configurations, :cleanup_empty_projects
+ end
+end
View
9 src/api/db/migrate/20140219185200_disable_publish_for_branches.rb
@@ -0,0 +1,9 @@
+class DisablePublishForBranches < ActiveRecord::Migration
+ def self.up
+ add_column :configurations, :disable_publish_for_branches, :boolean, :default => true
+ end
+
+ def self.down
+ remove_column :configurations, :disable_publish_for_branches
+ end
+end
View
6 src/api/db/structure.sql
@@ -344,6 +344,8 @@ CREATE TABLE `configurations` (
`theme` varchar(255) COLLATE utf8_bin DEFAULT NULL,
`obs_url` varchar(255) COLLATE utf8_bin DEFAULT NULL,
`cleanup_after_days` int(11) DEFAULT NULL,
+ `cleanup_empty_projects` tinyint(1) DEFAULT '1',
+ `disable_publish_for_branches` tinyint(1) DEFAULT '1',
PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin;
@@ -1442,6 +1444,10 @@ INSERT INTO schema_migrations (version) VALUES ('20140124071042');
INSERT INTO schema_migrations (version) VALUES ('20140210114542');
+INSERT INTO schema_migrations (version) VALUES ('20140218174400');
+
+INSERT INTO schema_migrations (version) VALUES ('20140219185200');
+
INSERT INTO schema_migrations (version) VALUES ('21');
INSERT INTO schema_migrations (version) VALUES ('22');
View
91 src/api/test/functional/branch_publish_flag_test.rb
@@ -0,0 +1,91 @@
+# encoding: UTF-8
+require File.expand_path(File.dirname(__FILE__) + "/..") + "/test_helper"
+require 'source_controller'
+
+class BranchPublishFlagTest < ActionDispatch::IntegrationTest
+
+ fixtures :all
+
+ def setup
+ super
+ wait_for_scheduler_start
+ end
+
+ @@verbose = false
+
+ def debug(message)
+ puts message if @@verbose
+ end
+
+ def branch_helper(parent_publish_allowed, expected_publish_allowed)
+ if parent_publish_allowed
+ # publish is not disabled for Apache
+ sprj = 'Apache'
+ spkg = 'apache2'
+ else
+ # publish is disabled for BinaryprotectedProject
+ sprj = 'BinaryprotectedProject'
+ spkg = 'bdpack'
+ end
+
+ tprj = "home:king:branches:#{sprj}"
+
+ debug "branching #{sprj}/#{spkg} into #{tprj}"
+ post "/source/#{sprj}/#{spkg}", :cmd => :branch, :target_project => "#{tprj}"
+ debug @response.body
+ assert_response :success
+ if @@verbose
+ debug "here is the branch:"
+ get "/source/#{tprj}"
+ debug @response.body
+ end
+ debug "fetching branch's meta:"
+ get "/source/#{tprj}/_meta"
+ debug @response.body
+
+ if expected_publish_allowed
+ # the XML says nothing about whether publishing is enabled, which means
+ # it is
+ assert_no_xml_tag :tag => "publish"
+ else
+ # publishing is explicitly disabled
+ assert_xml_tag :tag => "publish", :child => { :tag => "disable" }
+ end
+
+ # get rid of the branch so we can try again
+ debug "deleting branch"
+ delete "/source/#{tprj}"
+ debug @response.body
+ assert_response :success
+ end
+
+ def test_branching
+ # we use an admin user so we can twiddle the configuration
+ login_king
+
+ # by default, OBS expects to have thousands of users, so publishing new
+ # branches is disabled to save resources
+ branch_helper(true, false)
+ branch_helper(false, false)
+
+ # "small team" mode: resources are unconstrained so we might as well
+ # publish everything by default
+ debug "allowing publishing for branches"
+ put '/configuration?disable_publish_for_branches=off'
+ debug @response.body
+ assert_response :success
+ branch_helper(true, true)
+ # ... but if the parent project isn't published, neither is the branch
+ branch_helper(false, false)
+
+ # explicitly go back to the default and check that the result is still
+ # the same
+ debug "explicitly disallowing publishing for branches"
+ put '/configuration?disable_publish_for_branches=on'
+ debug @response.body
+ assert_response :success
+ branch_helper(true, false)
+ branch_helper(false, false)
+ end
+end
+
View
60 src/api/test/functional/request_controller_test.rb
@@ -2696,4 +2696,64 @@ def test_repository_delete_request
assert_response :success
end
+ def cleanup_empty_projects_helper(expect_cleanup_empty_project)
+ sprj = 'Apache'
+ bprj = "home:king:branches:#{sprj}"
+
+ post "/source/#{sprj}/apache2", :cmd => :branch, :target_project => "#{bprj}"
+ assert_response :success
+
+ post "/source/#{sprj}/Tidy", :cmd => :branch, :target_project => "#{bprj}"
+ assert_response :success
+
+ # Submit apache2 back. It is not the last project.
+ raw_post '/request?cmd=create', "<request><action type='submit'><source project='#{bprj}' package='apache2'/><target project='#{sprj}' package='apache2'/></action></request>"
+ assert_response :success
+ # Accept our own request :-)
+ id = Xmlhash.parse(@response.body)['id']
+ post "/request/#{id}?cmd=changestate&newstate=accepted"
+ assert_response :success
+ # apache2 has gone, but the project remains
+ get "/source/#{bprj}"
+ assert_response :success
+ assert_no_xml_tag tag: 'entry', attributes: { name: 'apache2' }
+ assert_xml_tag tag: 'entry', attributes: { name: 'Tidy' }
+
+ # Submit Tidy back. It *is* the last project.
+ raw_post '/request?cmd=create', "<request><action type='submit'><source project='#{bprj}' package='Tidy'/><target project='#{sprj}' package='Tidy'/></action></request>"
+ assert_response :success
+ id = Xmlhash.parse(@response.body)['id']
+ post "/request/#{id}?cmd=changestate&newstate=accepted"
+ assert_response :success
+ get "/source/#{bprj}"
+ if expect_cleanup_empty_project
+ assert_response 404
+ else
+ assert_response :success
+ assert_no_xml_tag tag: 'entry', attributes: { name: 'apache2' }
+ assert_no_xml_tag tag: 'entry', attributes: { name: 'Tidy' }
+ end
+ end
+
+ def test_cleanup_empty_projects
+ # we use an admin user so we can twiddle the configuration
+ login_king
+
+ # By default, OBS expects to have thousands of users, so succesfully
+ # submitting the last package in a project cleans up the project to
+ # save resources.
+ cleanup_empty_projects_helper(true)
+
+ # "small team" mode: resources are unconstrained so we're willing to
+ # preserve everyone's project configuration even if the project is empty
+ put '/configuration?cleanup_empty_projects=off'
+ assert_response :success
+ cleanup_empty_projects_helper(false)
+
+ # explicitly go back to the default and check that the result is still
+ # the same
+ put '/configuration?cleanup_empty_projects=on'
+ assert_response :success
+ cleanup_empty_projects_helper(true)
+ end
end
View
2  src/backend/BSXML.pm
@@ -1471,6 +1471,8 @@ our $configuration = [
'http_proxy',
'no_proxy',
'theme',
+ 'cleanup_empty_projects',
+ 'disable_publish_for_branches',
[ 'schedulers' =>
[ 'arch' ],
],
Something went wrong with that request. Please try again.