From dab95689a3f267e6ee4e4fa9da304a01d37a5a4e Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 23 Jun 2016 13:07:51 +0000 Subject: Fix `ref` parameter name for `commits/statuses` The attribute to filter by branch or tag needs to be named `ref`, not `ref_name`. And indeed the attribute in the JSON response is `ref` (and not `ref_name`). Tested on Gitlab CE 8.9. --- doc/api/commits.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/commits.md b/doc/api/commits.md index 57c2e1d9b87..d1317e5cc07 100644 --- a/doc/api/commits.md +++ b/doc/api/commits.md @@ -229,7 +229,7 @@ GET /projects/:id/repository/commits/:sha/statuses | --------- | ---- | -------- | ----------- | | `id` | integer | yes | The ID of a project | `sha` | string | yes | The commit SHA -| `ref_name`| string | no | The name of a repository branch or tag or, if not given, the default branch +| `ref` | string | no | The name of a repository branch or tag or, if not given, the default branch | `stage` | string | no | Filter by [build stage](../ci/yaml/README.md#stages), e.g., `test` | `name` | string | no | Filter by [job name](../ci/yaml/README.md#jobs), e.g., `bundler:audit` | `all` | boolean | no | Return all statuses, not only the latest ones -- cgit v1.2.1 From 33540238ee7da03ef532385e7a52103276252c18 Mon Sep 17 00:00:00 2001 From: Christian Meter Date: Thu, 7 Jul 2016 11:47:13 +0000 Subject: Replace builds_emails_service.png to fit to new layout of GitLab --- doc/project_services/img/builds_emails_service.png | Bin 33943 -> 30956 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/doc/project_services/img/builds_emails_service.png b/doc/project_services/img/builds_emails_service.png index 88943dc410e..440728795be 100644 Binary files a/doc/project_services/img/builds_emails_service.png and b/doc/project_services/img/builds_emails_service.png differ -- cgit v1.2.1 From 51ef233e25d6b9cfed16b8b3a5b532e429993dec Mon Sep 17 00:00:00 2001 From: Niels Keurentjes Date: Sun, 10 Jul 2016 23:35:10 +0000 Subject: Fix docker.sock reference in config.toml Mapping was omitted so it would just create a temp volume. --- doc/ci/docker/using_docker_build.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md index 7f83f846454..ee924323d96 100644 --- a/doc/ci/docker/using_docker_build.md +++ b/doc/ci/docker/using_docker_build.md @@ -188,7 +188,7 @@ In order to do that, follow the steps: image = "docker:latest" privileged = false disable_cache = false - volumes = ["/var/run/docker.sock", "/cache"] + volumes = ["/var/run/docker.sock:/var/run/docker.sock", "/cache"] [runners.cache] Insecure = false ``` -- cgit v1.2.1 From 24538ec0902667d399b47c7715aa75035aacecf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Libor=20Klep=C3=A1=C4=8D?= Date: Mon, 8 Aug 2016 16:28:57 +1000 Subject: Fix error 500 on Runners page. ~~~~ ActionView::Template::Error (Missing partial kaminari/_paginator ~~~~ Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=819903 Signed-off-by: Dmitry Smirnov --- app/views/admin/runners/index.html.haml | 2 +- app/views/admin/runners/show.html.haml | 2 +- app/views/projects/runners/_specific_runners.html.haml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/admin/runners/index.html.haml b/app/views/admin/runners/index.html.haml index a53876d6757..f68b6cba524 100644 --- a/app/views/admin/runners/index.html.haml +++ b/app/views/admin/runners/index.html.haml @@ -73,4 +73,4 @@ - @runners.each do |runner| = render "admin/runners/runner", runner: runner - = paginate @runners + = paginate @runners, theme: "gitlab" diff --git a/app/views/admin/runners/show.html.haml b/app/views/admin/runners/show.html.haml index 61abfc6ecbe..8ca2c97481c 100644 --- a/app/views/admin/runners/show.html.haml +++ b/app/views/admin/runners/show.html.haml @@ -67,7 +67,7 @@ = form_for [:admin, project.namespace.becomes(Namespace), project, project.runner_projects.new] do |f| = f.hidden_field :runner_id, value: @runner.id = f.submit 'Enable', class: 'btn btn-xs' - = paginate @projects + = paginate @projects, theme: "gitlab" .col-md-6 %h4 Recent builds served by this runner diff --git a/app/views/projects/runners/_specific_runners.html.haml b/app/views/projects/runners/_specific_runners.html.haml index d469dda5b81..4278980d1f4 100644 --- a/app/views/projects/runners/_specific_runners.html.haml +++ b/app/views/projects/runners/_specific_runners.html.haml @@ -26,4 +26,4 @@ %h4.underlined-title Available specific runners %ul.bordered-list.available-specific-runners = render partial: 'runner', collection: @assignable_runners, as: :runner - = paginate @assignable_runners + = paginate @assignable_runners, theme: "gitlab" -- cgit v1.2.1 From c920aad3ee82900c11f240f913abcd2caa0a3353 Mon Sep 17 00:00:00 2001 From: Wilfred Hughes Date: Mon, 3 Oct 2016 01:11:50 +0000 Subject: Grammar fixes in docs --- doc/ci/docker/using_docker_images.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/ci/docker/using_docker_images.md b/doc/ci/docker/using_docker_images.md index a849905ac6b..1007db3df1a 100644 --- a/doc/ci/docker/using_docker_images.md +++ b/doc/ci/docker/using_docker_images.md @@ -37,7 +37,7 @@ The registered runner will use the `ruby:2.1` docker image and will run two services, `postgres:latest` and `mysql:latest`, both of which will be accessible during the build process. -## What is image +## What is an image The `image` keyword is the name of the docker image that is present in the local Docker Engine (list all images with `docker images`) or any image that @@ -47,7 +47,7 @@ Hub please read the [Docker Fundamentals][] documentation. In short, with `image` we refer to the docker image, which will be used to create a container on which your build will run. -## What is service +## What is a service The `services` keyword defines just another docker image that is run during your build and is linked to the docker image that the `image` keyword defines. @@ -61,7 +61,7 @@ time the project is built. You can see some widely used services examples in the relevant documentation of [CI services examples](../services/README.md). -### How is service linked to the build +### How services are linked to the build To better understand how the container linking works, read [Linking containers together][linking-containers]. -- cgit v1.2.1 From 43438c3695ac06e0d12e24824f2c191f9608a51b Mon Sep 17 00:00:00 2001 From: Ismael Arenzana Date: Fri, 7 Oct 2016 17:29:48 +0000 Subject: Changed gitlab-shell version to avoid warning when precompiling the assets. --- doc/update/8.11-to-8.12.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/update/8.11-to-8.12.md b/doc/update/8.11-to-8.12.md index 07743d050f7..cddfa7e3e01 100644 --- a/doc/update/8.11-to-8.12.md +++ b/doc/update/8.11-to-8.12.md @@ -72,7 +72,7 @@ sudo -u git -H git checkout 8-12-stable-ee ```bash cd /home/git/gitlab-shell sudo -u git -H git fetch --all --tags -sudo -u git -H git checkout v3.6.0 +sudo -u git -H git checkout v3.6.1 ``` ### 6. Update gitlab-workhorse -- cgit v1.2.1 From 493367108eef14c8517c6d023ec46267c1e706cf Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Wed, 19 Oct 2016 14:30:17 +0200 Subject: Use MergeRequestsClosingIssues cache data on Issue#closed_by_merge_requests method --- CHANGELOG.md | 2 ++ app/models/issue.rb | 8 +++++++- spec/models/issue_spec.rb | 8 +++++++- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b018fc0d57..6c0fd97b070 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ Please view this file on the master branch, on stable branches it's out of date. ## 8.14.0 (2016-11-22) + - Use MergeRequestsClosingIssues cache data on Issue#closed_by_merge_requests method + ## 8.13.0 (2016-10-22) - Fix save button on project pipeline settings page. (!6955) diff --git a/app/models/issue.rb b/app/models/issue.rb index 133a5993815..89158a50353 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -211,7 +211,13 @@ class Issue < ActiveRecord::Base note.all_references(current_user, extractor: ext) end - ext.merge_requests.select { |mr| mr.open? && mr.closes_issue?(self) } + merge_requests = ext.merge_requests.select(&:open?) + if merge_requests.any? + ids = MergeRequestsClosingIssues.where(merge_request_id: merge_requests.map(&:id), issue_id: id).pluck(:merge_request_id) + merge_requests.select { |mr| mr.id.in?(ids) } + else + [] + end end def moved? diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 3b8b743af2d..60d30eb7418 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -100,11 +100,17 @@ describe Issue, models: true do end it 'returns the merge request to close this issue' do - allow(mr).to receive(:closes_issue?).with(issue).and_return(true) + mr expect(issue.closed_by_merge_requests).to eq([mr]) end + it "returns an empty array when the merge request is closed already" do + closed_mr + + expect(issue.closed_by_merge_requests).to eq([]) + end + it "returns an empty array when the current issue is closed already" do expect(closed_issue.closed_by_merge_requests).to eq([]) end -- cgit v1.2.1 From e9218c7e4e6c46d72d0860ba1107643a9f15f542 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Wed, 19 Oct 2016 10:04:05 +0200 Subject: Change target Ruby version for Rubocop to 2.1. We have to use the lowest common denominator to check the supported syntax and in our case it is Ruby 2.1. Please note that it will not help with unsupported syntax in HAML files because they are not checked by Rubocop. --- .rubocop.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.rubocop.yml b/.rubocop.yml index bec2464c740..13df3f99613 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -5,7 +5,7 @@ require: inherit_from: .rubocop_todo.yml AllCops: - TargetRubyVersion: 2.3 + TargetRubyVersion: 2.1 # Cop names are not d§splayed in offense messages by default. Change behavior # by overriding DisplayCopNames, or by giving the -D/--display-cop-names # option. -- cgit v1.2.1 From 23e81bfde879dfd116cb70cb8034d0fce21fffb6 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Thu, 20 Oct 2016 16:40:24 +0200 Subject: Ignore external issues when bulk assigning issues to author of merge request. Fixes #23552 --- app/services/merge_requests/assign_issues_service.rb | 2 +- spec/services/merge_requests/assign_issues_service_spec.rb | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/services/merge_requests/assign_issues_service.rb b/app/services/merge_requests/assign_issues_service.rb index f636e5fec4f..066efa1acc3 100644 --- a/app/services/merge_requests/assign_issues_service.rb +++ b/app/services/merge_requests/assign_issues_service.rb @@ -4,7 +4,7 @@ module MergeRequests @assignable_issues ||= begin if current_user == merge_request.author closes_issues.select do |issue| - !issue.assignee_id? && can?(current_user, :admin_issue, issue) + !issue.is_a?(ExternalIssue) && !issue.assignee_id? && can?(current_user, :admin_issue, issue) end else [] diff --git a/spec/services/merge_requests/assign_issues_service_spec.rb b/spec/services/merge_requests/assign_issues_service_spec.rb index 7aeb95a15ea..5034b6ef33f 100644 --- a/spec/services/merge_requests/assign_issues_service_spec.rb +++ b/spec/services/merge_requests/assign_issues_service_spec.rb @@ -46,4 +46,16 @@ describe MergeRequests::AssignIssuesService, services: true do it 'assigns these to the merge request owner' do expect { service.execute }.to change { issue.reload.assignee }.to(user) end + + it 'ignores external issues' do + external_issue = ExternalIssue.new('JIRA-123', project) + service = described_class.new( + project, + user, + merge_request: merge_request, + closes_issues: [external_issue] + ) + + expect(service.assignable_issues.count).to eq 0 + end end -- cgit v1.2.1 From f956de7c3991f229967339ad04aa5464c02b3ac6 Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Thu, 20 Oct 2016 20:16:35 +0200 Subject: Refactor and add new functionality to CI yaml reference [ci ski] --- doc/ci/yaml/README.md | 160 ++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 135 insertions(+), 25 deletions(-) diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 84ea59ab687..5c0e1c44e3f 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -146,13 +146,17 @@ variables: ``` These variables can be later used in all executed commands and scripts. - The YAML-defined variables are also set to all created service containers, -thus allowing to fine tune them. +thus allowing to fine tune them. Variables can be also defined on a +[job level](#job-variables). -Variables can be also defined on [job level](#job-variables). +Except for the user defined variables, there are also the ones set up by the +Runner itself. One example would be `CI_BUILD_REF_NAME` which has the value of +the branch or tag name for which project is built. Apart from the variables +you can set in `.gitlab-ci.yml`, there are also the so called secret variables +which can be set in GitLab's UI. -[Learn more about variables.](../variables/README.md) +[Learn more about variables.][variables] ### cache @@ -541,20 +545,29 @@ An example usage of manual actions is deployment to production. > Introduced in GitLab 8.9. -`environment` is used to define that a job deploys to a specific [environment]. -This allows easy tracking of all deployments to your environments straight from -GitLab. +> You can read more about environments and find more examples in the +[documentation about environments][environment]. +`environment` is used to define that a job deploys to a specific environment. If `environment` is specified and no environment under that name exists, a new one will be created automatically. -The `environment` name must contain only letters, digits, '-', '_', '/', '$', '{', '}' and spaces. Common -names are `qa`, `staging`, and `production`, but you can use whatever name works -with your workflow. +The `environment` name can contain: ---- +- letters +- digits +- spaces +- `-` +- `_` +- `/` +- `$` +- `{` +- `}` -**Example configurations** +Common names are `qa`, `staging`, and `production`, but you can use whatever +name works with your workflow. + +In its simplest form, the `environment` keyword can be defined like: ``` deploy to production: @@ -563,39 +576,134 @@ deploy to production: environment: production ``` -The `deploy to production` job will be marked as doing deployment to -`production` environment. +In the above example, the `deploy to production` job will be marked as doing a +deployment to the `production` environment. + +#### environment:name + +> Introduced in GitLab 8.11. + +>**Note:** +Before GitLab 8.11, the name of an environment could be defined as a string like +`environment: production`. The recommended way now is to define it under the +`name` keyword. + +Instead of defining the name of the environment right after the `environment` +keyword, it is also possible to define it as a separate value. For that, use +the `name` keyword under `environment`: + +``` +deploy to production: + stage: deploy + script: git push production HEAD:master + environment: + name: production +``` + +#### environment:url + +> Introduced in GitLab 8.11. + +>**Note:** +Before GitLab 8.11, the URL could be added only in GitLab's UI. The +recommended way now is to define it in `.gitlab-ci.yml`. + +This is an optional value that when set, it exposes buttons in various places +in GitLab which when clicked take you to the defined URL. + +In the example below, if the job finishes successfully, it will create buttons +in the merge requests and in the environments/deployments pages which will point +to `https://prod.example.com`. + +``` +deploy to production: + stage: deploy + script: git push production HEAD:master + environment: + name: production + url: https://prod.example.com +``` + +#### environment:on_stop + +> [Introduced][ce-6669] in GitLab 8.13. + +Closing (stoping) environments can be achieved with the `on_stop` keyword defined under +`environment`. It declares a different job that runs in order to close +the environment. + +Read the `environment:action` section for an example. + +#### environment:action + +> [Introduced][ce-6669] in GitLab 8.13. + +The `action` keyword is to be used in conjunction with `on_stop` and is defined +in the job that is called to close the environment. + +Take for instance: + +```yaml +review_app: + stage: deploy + script: make deploy-app + environment: + name: review + on_stop: stop_review_app + +stop_review_app: + stage: deploy + script: make delete-app + when: manual + environment: + name: review + action: stop +``` + +In the above example we set up the `review_app` job to deploy to the `review` +environment, and we also defined a new `stop_review_app` job under `on_stop`. +Once the `review_app` job is successfully finished, it will trigger the +`stop_review_app` job based on what is defined under `when`. In this case we +set it up to `manual` so it will need a [manual action](#manual-actions) via +GitLab's web interface in order to run. + +The `stop_review_app` job is **required** to have the following keywords defined: + +- `when` - [reference](#when) +- `environment:name` +- `environment:action` #### dynamic environments > [Introduced][ce-6323] in GitLab 8.12 and GitLab Runner 1.6. `environment` can also represent a configuration hash with `name` and `url`. -These parameters can use any of the defined CI [variables](#variables) +These parameters can use any of the defined [CI variables](#variables) (including predefined, secure variables and `.gitlab-ci.yml` variables). -The common use case is to create dynamic environments for branches and use them -as review apps. - ---- - -**Example configurations** +For example: ``` deploy as review app: stage: deploy - script: ... + script: make deploy environment: name: review-apps/$CI_BUILD_REF_NAME url: https://$CI_BUILD_REF_NAME.review.example.com/ ``` The `deploy as review app` job will be marked as deployment to dynamically -create the `review-apps/branch-name` environment. +create the `review-apps/$CI_BUILD_REF_NAME` environment, which `$CI_BUILD_REF_NAME` +is an [environment variable][variables] set by the Runner. If for example the +`deploy as review app` job was run in a branch named `pow`, this environment +should be accessible under `https://pow.review.example.com/`. -This environment should be accessible under `https://branch-name.review.example.com/`. +This of course implies that the underlying server which hosts the application +is properly configured. -You can see a simple example at https://gitlab.com/gitlab-examples/review-apps-nginx/. +The common use case is to create dynamic environments for branches and use them +as Review Apps. You can see a simple example using Review Apps at +https://gitlab.com/gitlab-examples/review-apps-nginx/. ### artifacts @@ -1105,3 +1213,5 @@ CI with various languages. [examples]: ../examples/README.md [ce-6323]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6323 [environment]: ../environments.md +[ce-6669]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6669 +[variables]: ../variables/README.md -- cgit v1.2.1 From 4e03f4c40602b568cffd591dcd5af6bd4b9a281e Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 19 Oct 2016 14:57:42 +0100 Subject: Fixed issues with sticky mr tabs & sidebar Closes #23504 --- app/assets/javascripts/merge_request_tabs.js | 11 +--- app/assets/stylesheets/framework/sidebar.scss | 8 +++ app/assets/stylesheets/pages/merge_requests.scss | 13 ++++- app/views/projects/merge_requests/_show.html.haml | 68 ++++++++++++----------- 4 files changed, 54 insertions(+), 46 deletions(-) diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index fd21aa1fefa..1a04a037210 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -388,8 +388,7 @@ // So we dont affix the tabs on these if (Breakpoints.get().getBreakpointSize() === 'xs' || !$tabs.length) return; - var tabsWidth = $tabs.outerWidth(), - $diffTabs = $('#diff-notes-app'), + var $diffTabs = $('#diff-notes-app'), offsetTop = $tabs.offset().top - ($('.navbar-fixed-top').height() + $('.layout-nav').height()); $tabs.off('affix.bs.affix affix-top.bs.affix') @@ -398,18 +397,10 @@ top: offsetTop } }).on('affix.bs.affix', function () { - $tabs.css({ - left: $tabs.offset().left, - width: tabsWidth - }); $diffTabs.css({ marginTop: $tabs.height() }); }).on('affix-top.bs.affix', function () { - $tabs.css({ - left: '', - width: '' - }); $diffTabs.css({ marginTop: '' }); diff --git a/app/assets/stylesheets/framework/sidebar.scss b/app/assets/stylesheets/framework/sidebar.scss index ec52f326eb9..1d8e64a0e4b 100644 --- a/app/assets/stylesheets/framework/sidebar.scss +++ b/app/assets/stylesheets/framework/sidebar.scss @@ -185,6 +185,10 @@ header.header-sidebar-pinned { @media (min-width: $screen-sm-min) { padding-right: $sidebar_collapsed_width; + + .merge-request-tabs-holder.affix { + right: $sidebar_collapsed_width; + } } .sidebar-collapsed-icon { @@ -207,6 +211,10 @@ header.header-sidebar-pinned { @media (min-width: $screen-md-min) { padding-right: $gutter_width; + + .merge-request-tabs-holder.affix { + right: $gutter_width; + } } &.with-overlay { diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 35a1877df95..70afa568554 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -183,11 +183,11 @@ .ci-coverage { float: right; } - + .stop-env-container { color: $gl-text-color; float: right; - + a { color: $gl-text-color; } @@ -438,11 +438,18 @@ } } -.merge-request-tabs { +.merge-request-tabs-holder { background-color: #fff; &.affix { top: 100px; + left: 0; z-index: 9; + transition: right .15s; + } + + &:not(.affix) .container-fluid { + padding-left: 0; + padding-right: 0; } } diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 0e19d224fcd..f57abe73977 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -47,39 +47,41 @@ = link_to "command line", "#modal_merge_info", class: "how_to_merge_link vlink", title: "How To Merge", "data-toggle" => "modal" - if @commits_count.nonzero? - %ul.merge-request-tabs.nav-links.no-top.no-bottom{ class: ("js-tabs-affix" unless ENV['RAILS_ENV'] == 'test') } - %li.notes-tab - = link_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#notes', action: 'notes', toggle: 'tab' } do - Discussion - %span.badge= @merge_request.mr_and_commit_notes.user.count - - if @merge_request.source_project - %li.commits-tab - = link_to commits_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#commits', action: 'commits', toggle: 'tab' } do - Commits - %span.badge= @commits_count - - if @pipeline - %li.pipelines-tab - = link_to pipelines_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: '#pipelines', action: 'pipelines', toggle: 'tab' } do - Pipelines - %span.badge= @pipelines.size - %li.builds-tab - = link_to builds_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: '#builds', action: 'builds', toggle: 'tab' } do - Builds - %span.badge= @statuses.size - %li.diffs-tab - = link_to diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#diffs', action: 'diffs', toggle: 'tab' } do - Changes - %span.badge= @merge_request.diff_size - %li#resolve-count-app.line-resolve-all-container.pull-right.prepend-top-10.hidden-xs{ "v-cloak" => true } - %resolve-count{ "inline-template" => true, ":logged-out" => "#{current_user.nil?}" } - .line-resolve-all{ "v-show" => "discussionCount > 0", - ":class" => "{ 'has-next-btn': !loggedOut && resolvedDiscussionCount !== discussionCount }" } - %span.line-resolve-btn.is-disabled{ type: "button", - ":class" => "{ 'is-active': resolvedDiscussionCount === discussionCount }" } - = render "shared/icons/icon_status_success.svg" - %span.line-resolve-text - {{ resolvedDiscussionCount }}/{{ discussionCount }} {{ discussionCount | pluralize 'discussion' }} resolved - = render "discussions/jump_to_next" + .merge-request-tabs-holder{ class: ("js-tabs-affix" unless ENV['RAILS_ENV'] == 'test') } + %div{ class: container_class } + %ul.merge-request-tabs.nav-links.no-top.no-bottom + %li.notes-tab + = link_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#notes', action: 'notes', toggle: 'tab' } do + Discussion + %span.badge= @merge_request.mr_and_commit_notes.user.count + - if @merge_request.source_project + %li.commits-tab + = link_to commits_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#commits', action: 'commits', toggle: 'tab' } do + Commits + %span.badge= @commits_count + - if @pipeline + %li.pipelines-tab + = link_to pipelines_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: '#pipelines', action: 'pipelines', toggle: 'tab' } do + Pipelines + %span.badge= @pipelines.size + %li.builds-tab + = link_to builds_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: '#builds', action: 'builds', toggle: 'tab' } do + Builds + %span.badge= @statuses.size + %li.diffs-tab + = link_to diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#diffs', action: 'diffs', toggle: 'tab' } do + Changes + %span.badge= @merge_request.diff_size + %li#resolve-count-app.line-resolve-all-container.pull-right.prepend-top-10.hidden-xs{ "v-cloak" => true } + %resolve-count{ "inline-template" => true, ":logged-out" => "#{current_user.nil?}" } + .line-resolve-all{ "v-show" => "discussionCount > 0", + ":class" => "{ 'has-next-btn': !loggedOut && resolvedDiscussionCount !== discussionCount }" } + %span.line-resolve-btn.is-disabled{ type: "button", + ":class" => "{ 'is-active': resolvedDiscussionCount === discussionCount }" } + = render "shared/icons/icon_status_success.svg" + %span.line-resolve-text + {{ resolvedDiscussionCount }}/{{ discussionCount }} {{ discussionCount | pluralize 'discussion' }} resolved + = render "discussions/jump_to_next" .tab-content#diff-notes-app #notes.notes.tab-pane.voting_notes -- cgit v1.2.1 From a28371dbe33c568c970c704b90760d2b540256af Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 19 Oct 2016 17:24:24 +0100 Subject: Fixed issue when images are loading it would push off the tabs --- app/assets/javascripts/merge_request_tabs.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 1a04a037210..9f28738e06b 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -389,12 +389,18 @@ if (Breakpoints.get().getBreakpointSize() === 'xs' || !$tabs.length) return; var $diffTabs = $('#diff-notes-app'), - offsetTop = $tabs.offset().top - ($('.navbar-fixed-top').height() + $('.layout-nav').height()); + $fixedNav = $('.navbar-fixed-top'), + $layoutNav = $('.layout-nav'); $tabs.off('affix.bs.affix affix-top.bs.affix') .affix({ offset: { - top: offsetTop + top: function () { + var tabsTop = $diffTabs.offset().top - $tabs.height(); + tabsTop = tabsTop - ($fixedNav.height() + $layoutNav.height()); + + return tabsTop; + } } }).on('affix.bs.affix', function () { $diffTabs.css({ -- cgit v1.2.1 From 57046eb0abf280594d6625db3429f13d45499c83 Mon Sep 17 00:00:00 2001 From: Bryce Johnson Date: Wed, 19 Oct 2016 14:40:58 +0200 Subject: Ensure custom provider tab labels don't break layout. (Also fix some issues for session views on small screens.) --- app/assets/stylesheets/pages/login.scss | 34 +++++++++++++++++++++++++- app/views/devise/sessions/two_factor.html.haml | 2 +- app/views/devise/shared/_tabs_ldap.html.haml | 2 +- 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/pages/login.scss b/app/assets/stylesheets/pages/login.scss index bdb13bee178..9496234c773 100644 --- a/app/assets/stylesheets/pages/login.scss +++ b/app/assets/stylesheets/pages/login.scss @@ -143,6 +143,7 @@ &:not(.active) { background-color: $gray-light; + border-left: 1px solid $border-color; } a { @@ -170,6 +171,31 @@ } } + // Ldap configurations may need more tabs & the tab labels are user generated (arbitrarily long). + // These styles prevent this from breaking the layout, and only applied when providers are configured. + + .new-session-tabs.custom-provider-tabs { + flex-wrap: wrap; + + li { + min-width: 85px; + flex-basis: auto; + + // This styles tab elements that have wrapped to a second line. We cannot easily predict when this will happen. + // We are making somewhat of an assumption about the configuration here: that users do not have more than + // 3 LDAP servers configured (in addition to standard login) and they are not using especially long names for any + // of them. If either condition is false, this will work as expected. If both are true, there may be a missing border + // above one of the bottom row elements. If you know a better way, please implement it! + &:nth-child(n+5) { + border-top: 1px solid $border-color; + } + } + + a { + font-size: 16px; + } + } + .form-control { &:active, &:focus { @@ -203,6 +229,7 @@ .login-page { .col-sm-5.pull-right { float: none !important; + margin-bottom: 45px; } } } @@ -244,7 +271,11 @@ } .navless-container { - padding: 65px; // height of footer + bottom padding of email confirmation link + padding: 65px 15px; // height of footer + bottom padding of email confirmation link + + @media (max-width: $screen-xs-max) { + padding: 0 15px 65px; + } } } @@ -263,3 +294,4 @@ bottom: 0; } } + diff --git a/app/views/devise/sessions/two_factor.html.haml b/app/views/devise/sessions/two_factor.html.haml index 0e865b807c1..fd77cdbee2e 100644 --- a/app/views/devise/sessions/two_factor.html.haml +++ b/app/views/devise/sessions/two_factor.html.haml @@ -10,7 +10,7 @@ = form_for(resource, as: resource_name, url: session_path(resource_name), method: :post, html: { class: 'edit_user show-gl-field-errors' }) do |f| - resource_params = params[resource_name].presence || params = f.hidden_field :remember_me, value: resource_params.fetch(:remember_me, 0) - .form-group + %div = f.label 'Two-Factor Authentication code', name: :otp_attempt = f.text_field :otp_attempt, class: 'form-control', required: true, autofocus: true, autocomplete: 'off', title: 'This field is required.' %p.help-block.hint Enter the code from the two-factor app on your mobile device. If you've lost your device, you may enter one of your recovery codes. diff --git a/app/views/devise/shared/_tabs_ldap.html.haml b/app/views/devise/shared/_tabs_ldap.html.haml index a057f126c45..1e957f0935f 100644 --- a/app/views/devise/shared/_tabs_ldap.html.haml +++ b/app/views/devise/shared/_tabs_ldap.html.haml @@ -1,4 +1,4 @@ -%ul.new-session-tabs.nav-links.nav-tabs +%ul.new-session-tabs.nav-links.nav-tabs{ class: ('custom-provider-tabs' if form_based_providers.any?) } - if crowd_enabled? %li.active = link_to "Crowd", "#crowd", 'data-toggle' => 'tab' -- cgit v1.2.1 From 1c668b125e8fd3e2959d0a2bd83447f09ea7fee4 Mon Sep 17 00:00:00 2001 From: blackst0ne Date: Fri, 21 Oct 2016 09:35:17 +1100 Subject: Add hover to trash icon in notes --- CHANGELOG.md | 1 + app/views/projects/notes/_note.html.haml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 73dc323e02c..128f5dec039 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date. ## 8.14.0 (2016-11-22) - Adds user project membership expired event to clarify why user was removed (Callum Dryden) - Fix HipChat notifications rendering (airatshigapov, eisnerd) + - Add hover to trash icon in notes !7008 (blackst0ne) - Simpler arguments passed to named_route on toggle_award_url helper method ## 8.13.0 (2016-10-22) diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index 73fe6a715fa..ab719e38904 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -57,7 +57,7 @@ = link_to '#', title: 'Edit comment', class: 'note-action-button js-note-edit' do = icon('pencil', class: 'link-highlight') = link_to namespace_project_note_path(note.project.namespace, note.project, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'note-action-button hidden-xs js-note-delete danger' do - = icon('trash-o') + = icon('trash-o', class: 'danger-highlight') .note-body{class: note_editable ? 'js-task-list-container' : ''} .note-text.md = preserve do -- cgit v1.2.1 From f87124da1f3cf48415457b2c8bdae7ce4cb573ea Mon Sep 17 00:00:00 2001 From: tauriedavis Date: Fri, 14 Oct 2016 11:42:08 -0700 Subject: fix font weight of project feature settings --- app/assets/stylesheets/pages/projects.scss | 16 +++- app/views/projects/edit.html.haml | 116 ++++++++++++++--------------- 2 files changed, 72 insertions(+), 60 deletions(-) diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 1062d7effb0..fe7cf3c87e3 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -13,9 +13,18 @@ .new_project, .edit-project { + fieldset { - &.features .control-label { - font-weight: normal; + + &.features { + + .label-light { + margin-bottom: 0; + } + + .help-block { + margin-top: 0; + } } .form-group { @@ -40,6 +49,7 @@ } .input-group > div { + &:last-child { padding-right: 0; } @@ -47,6 +57,7 @@ @media (max-width: $screen-xs-max) { .input-group > div { + margin-bottom: 14px; &:last-child { @@ -60,6 +71,7 @@ } .input-group-addon { + &.static-namespace { height: 35px; border-radius: 3px; diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index fb776e3a3e7..55b6580e640 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -46,70 +46,70 @@ %h5.prepend-top-0 Feature Visibility - = f.fields_for :project_feature do |feature_fields| - .form_group.prepend-top-20 - .row - .col-md-9 - = feature_fields.label :repository_access_level, "Repository", class: 'label-light' - %span.help-block Push files to be stored in this project - .col-md-3.js-repo-access-level - = project_feature_access_select(:repository_access_level) + = f.fields_for :project_feature do |feature_fields| + .form_group.prepend-top-20 + .row + .col-md-9 + = feature_fields.label :repository_access_level, "Repository", class: 'label-light' + %span.help-block Push files to be stored in this project + .col-md-3.js-repo-access-level + = project_feature_access_select(:repository_access_level) - .col-sm-12 - .row - .col-md-9.project-feature-nested - = feature_fields.label :merge_requests_access_level, "Merge requests", class: 'label-light' - %span.help-block Submit changes to be merged upstream - .col-md-3 - = project_feature_access_select(:merge_requests_access_level) + .col-sm-12 + .row + .col-md-9.project-feature-nested + = feature_fields.label :merge_requests_access_level, "Merge requests", class: 'label-light' + %span.help-block Submit changes to be merged upstream + .col-md-3 + = project_feature_access_select(:merge_requests_access_level) - .row - .col-md-9.project-feature-nested - = feature_fields.label :builds_access_level, "Builds", class: 'label-light' - %span.help-block Submit, test and deploy your changes before merge - .col-md-3 - = project_feature_access_select(:builds_access_level) + .row + .col-md-9.project-feature-nested + = feature_fields.label :builds_access_level, "Builds", class: 'label-light' + %span.help-block Submit, test and deploy your changes before merge + .col-md-3 + = project_feature_access_select(:builds_access_level) - .row - .col-md-9 - = feature_fields.label :snippets_access_level, "Snippets", class: 'label-light' - %span.help-block Share code pastes with others out of Git repository - .col-md-3 - = project_feature_access_select(:snippets_access_level) + .row + .col-md-9 + = feature_fields.label :snippets_access_level, "Snippets", class: 'label-light' + %span.help-block Share code pastes with others out of Git repository + .col-md-3 + = project_feature_access_select(:snippets_access_level) - .row - .col-md-9 - = feature_fields.label :issues_access_level, "Issues", class: 'label-light' - %span.help-block Lightweight issue tracking system for this project - .col-md-3 - = project_feature_access_select(:issues_access_level) + .row + .col-md-9 + = feature_fields.label :issues_access_level, "Issues", class: 'label-light' + %span.help-block Lightweight issue tracking system for this project + .col-md-3 + = project_feature_access_select(:issues_access_level) - .row - .col-md-9 - = feature_fields.label :wiki_access_level, "Wiki", class: 'label-light' - %span.help-block Pages for project documentation - .col-md-3 - = project_feature_access_select(:wiki_access_level) - - - if Gitlab.config.lfs.enabled && current_user.admin? - .checkbox - = f.label :lfs_enabled do - = f.check_box :lfs_enabled - %strong LFS - %br - %span.descr - Git Large File Storage - = link_to icon('question-circle'), help_page_path('workflow/lfs/manage_large_binaries_with_git_lfs') + .row + .col-md-9 + = feature_fields.label :wiki_access_level, "Wiki", class: 'label-light' + %span.help-block Pages for project documentation + .col-md-3 + = project_feature_access_select(:wiki_access_level) - if Gitlab.config.lfs.enabled && current_user.admin? - .form-group - .checkbox - = f.label :container_registry_enabled do - = f.check_box :container_registry_enabled - %strong Container Registry - %br - %span.descr Enable Container Registry for this project - = link_to icon('question-circle'), help_page_path('user/project/container_registry'), target: '_blank' + .checkbox + = f.label :lfs_enabled do + = f.check_box :lfs_enabled + %strong LFS + %br + %span.descr + Git Large File Storage + = link_to icon('question-circle'), help_page_path('workflow/lfs/manage_large_binaries_with_git_lfs') + + - if Gitlab.config.lfs.enabled && current_user.admin? + .form-group + .checkbox + = f.label :container_registry_enabled do + = f.check_box :container_registry_enabled + %strong Container Registry + %br + %span.descr Enable Container Registry for this project + = link_to icon('question-circle'), help_page_path('user/project/container_registry'), target: '_blank' = render 'merge_request_settings', f: f %hr @@ -288,4 +288,4 @@ Saving project. %p Please wait a moment, this page will automatically refresh when ready. -= render 'shared/confirm_modal', phrase: @project.path += render 'shared/confirm_modal', phrase: @project.path \ No newline at end of file -- cgit v1.2.1 From 168197cd5a179c961301225626ac1a175f892782 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Tue, 18 Oct 2016 16:49:19 +0300 Subject: Fix project member access levels --- CHANGELOG.md | 1 + .../20161018124658_make_project_owners_masters.rb | 15 +++++++++ db/schema.rb | 2 +- .../projects/project_members_controller_spec.rb | 36 ++++++++++++++++++++++ spec/requests/api/members_spec.rb | 11 +++++++ 5 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20161018124658_make_project_owners_masters.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 670404e4fce..16ca2ff93e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -137,6 +137,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Fix buggy iOS tooltip layering behavior. - Make guests unable to view MRs on private projects - Fix broken Project API docs (Takuya Noguchi) + - Migrate invalid project members (owner -> master) ## 8.12.7 diff --git a/db/migrate/20161018124658_make_project_owners_masters.rb b/db/migrate/20161018124658_make_project_owners_masters.rb new file mode 100644 index 00000000000..a576bb7b622 --- /dev/null +++ b/db/migrate/20161018124658_make_project_owners_masters.rb @@ -0,0 +1,15 @@ +class MakeProjectOwnersMasters < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + update_column_in_batches(:members, :access_level, 40) do |table, query| + query.where(table[:access_level].eq(50).and(table[:source_type].eq('Project'))) + end + end + + def down + # do nothing + end +end diff --git a/db/schema.rb b/db/schema.rb index a3c7fc2fd57..f5c01511195 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -843,7 +843,7 @@ ActiveRecord::Schema.define(version: 20161019213545) do t.integer "builds_access_level" t.datetime "created_at" t.datetime "updated_at" - t.integer "repository_access_level", default: 20, null: false + t.integer "repository_access_level", default: 20, null: false end add_index "project_features", ["project_id"], name: "index_project_features_on_project_id", using: :btree diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 8519ebc1d5f..5e487241d07 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -228,4 +228,40 @@ describe Projects::ProjectMembersController do end end end + + describe 'POST create' do + let(:stranger) { create(:user) } + + context 'when creating owner' do + before do + project.team << [user, :master] + sign_in(user) + end + + it 'does not create a member' do + expect do + post :create, user_ids: stranger.id, + namespace_id: project.namespace, + access_level: Member::OWNER, + project_id: project + end.to change { project.members.count }.by(0) + end + end + + context 'when create master' do + before do + project.team << [user, :master] + sign_in(user) + end + + it 'creates a member' do + expect do + post :create, user_ids: stranger.id, + namespace_id: project.namespace, + access_level: Member::MASTER, + project_id: project + end.to change { project.members.count }.by(1) + end + end + end end diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index d22e0595788..493c0a893d1 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -328,4 +328,15 @@ describe API::Members, api: true do it_behaves_like 'DELETE /:sources/:id/members/:user_id', 'group' do let(:source) { group } end + + context 'Adding owner to project' do + it 'returns 403' do + expect do + post api("/projects/#{project.id}/members", master), + user_id: stranger.id, access_level: Member::OWNER + + expect(response).to have_http_status(422) + end.to change { project.members.count }.by(0) + end + end end -- cgit v1.2.1 From b332931af375a29fa0ff41d45315400beca44c7a Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Thu, 20 Oct 2016 21:43:24 -0700 Subject: Fix broken label uniqueness label migration The previous implementation of the migration failed on staging because the migration was attempted to remove labels from projects that did not actually have duplicates. This occurred because the SQL query did not account for the project ID when selecting the labels. To replicate the problem: 1. Disable the uniqueness validation in app/models/label.rb. 2. Create a duplicate label "bug" in project A. 3. Create the same label in project B with label "bug". The migration will attempt to remove the label in B even if there are no duplicates. Closes #23609 --- db/migrate/20161017125927_add_unique_index_to_labels.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/db/migrate/20161017125927_add_unique_index_to_labels.rb b/db/migrate/20161017125927_add_unique_index_to_labels.rb index 16ae38612de..f2b56ebfb7b 100644 --- a/db/migrate/20161017125927_add_unique_index_to_labels.rb +++ b/db/migrate/20161017125927_add_unique_index_to_labels.rb @@ -7,9 +7,9 @@ class AddUniqueIndexToLabels < ActiveRecord::Migration disable_ddl_transaction! def up - select_all('SELECT title, COUNT(id) as cnt FROM labels GROUP BY project_id, title HAVING COUNT(id) > 1').each do |label| + select_all('SELECT title, project_id, COUNT(id) as cnt FROM labels GROUP BY project_id, title HAVING COUNT(id) > 1').each do |label| label_title = quote_string(label['title']) - duplicated_ids = select_all("SELECT id FROM labels WHERE title = '#{label_title}' ORDER BY id ASC").map{ |label| label['id'] } + duplicated_ids = select_all("SELECT id FROM labels WHERE project_id = #{label['project_id']} AND title = '#{label_title}' ORDER BY id ASC").map{ |label| label['id'] } label_id = duplicated_ids.first duplicated_ids.delete(label_id) -- cgit v1.2.1 From c81ff152e08d58c13efbd50c40dd2e083ac65083 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Fri, 21 Oct 2016 13:53:38 +0200 Subject: Change "Group#web_url" to return "/groups/twitter" rather than "/twitter". Bring back the old behaviour which was changed by 6b90ccb9. Fixes #23527. --- app/models/group.rb | 2 +- config/routes/group.rb | 33 ++++++++++++++++++--------------- spec/models/group_spec.rb | 6 ++++++ 3 files changed, 25 insertions(+), 16 deletions(-) diff --git a/app/models/group.rb b/app/models/group.rb index 00a595d2705..6865e610718 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -68,7 +68,7 @@ class Group < Namespace end def web_url - Gitlab::Routing.url_helpers.group_url(self) + Gitlab::Routing.url_helpers.group_canonical_url(self) end def human_name diff --git a/config/routes/group.rb b/config/routes/group.rb index 4838c9d91c6..826048ba196 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -12,23 +12,26 @@ constraints(GroupUrlConstrainer.new) do end end -resources :groups, constraints: { id: /[a-zA-Z.0-9_\-]+(? 'groups#show', as: :group_canonical end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index ac862055ebc..47f89f744cb 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -265,4 +265,10 @@ describe Group, models: true do members end + + describe '#web_url' do + it 'returns the canonical URL' do + expect(group.web_url).to include("groups/#{group.name}") + end + end end -- cgit v1.2.1 From a74dfa301e77752e333467892073811677e82c89 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 21 Oct 2016 13:43:52 +0100 Subject: Fixed compare ellipsis messing with layout --- app/views/projects/compare/_form.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/compare/_form.html.haml b/app/views/projects/compare/_form.html.haml index 76b68c544aa..7bde20c3286 100644 --- a/app/views/projects/compare/_form.html.haml +++ b/app/views/projects/compare/_form.html.haml @@ -10,7 +10,7 @@ = button_tag type: 'button', class: "form-control compare-dropdown-toggle js-compare-dropdown", required: true, data: { refs_url: refs_namespace_project_path(@project.namespace, @project), toggle: "dropdown", target: ".js-compare-from-dropdown", selected: params[:from], field_name: :from } do .dropdown-toggle-text= params[:from] || 'Select branch/tag' = render "ref_dropdown" - .compare-ellipsis ... + .compare-ellipsis.inline ... .form-group.dropdown.compare-form-group.to.js-compare-to-dropdown .input-group.inline-input-group %span.input-group-addon to -- cgit v1.2.1 From 1a53511a3454bf70786d72e59530bff42ae160e4 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Fri, 14 Oct 2016 16:29:54 -0500 Subject: Fix object data to be sent to fetch analytics data --- app/assets/javascripts/cycle_analytics.js.es6 | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/cycle_analytics.js.es6 b/app/assets/javascripts/cycle_analytics.js.es6 index bd9accacb8c..20791bab942 100644 --- a/app/assets/javascripts/cycle_analytics.js.es6 +++ b/app/assets/javascripts/cycle_analytics.js.es6 @@ -36,7 +36,11 @@ method: 'GET', dataType: 'json', contentType: 'application/json', - data: { start_date: options.startDate } + data: { + cycle_analytics: { + start_date: options.startDate + } + } }).done((data) => { this.decorateData(data); this.initDropdown(); -- cgit v1.2.1 From 6c2ab27aeaf0cd59d87e14876492a4162d48e2d7 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Wed, 19 Oct 2016 10:27:24 -0500 Subject: Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c5c96c4528..42e3df435bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Update duration at the end of pipeline - ExpireBuildArtifactsWorker query builds table without ordering enqueuing one job per build to cleanup - Add group level labels. (!6425) + - Fix Cycle analytics not showing correct data when filtering by date. !6906 - Add an example for testing a phoenix application with Gitlab CI in the docs (Manthan Mallikarjun) - Cancelled pipelines could be retried. !6927 - Updating verbiage on git basics to be more intuitive -- cgit v1.2.1 From b939529c2a2c724f1471ab3b0ec2a5dac10c913c Mon Sep 17 00:00:00 2001 From: Airat Shigapov Date: Fri, 21 Oct 2016 18:05:36 +0300 Subject: Fix wrong endpoint in api/users documentation, fix same typo in spec describe blocks --- doc/api/users.md | 2 +- spec/requests/api/users_spec.rb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/api/users.md b/doc/api/users.md index 2b12770d5a5..a50ba5432fe 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -643,7 +643,7 @@ Parameters: | `id` | integer | yes | The ID of the user | ```bash -curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/user/:id/events +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/users/:id/events ``` Example response: diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index f83f4d2c9b1..d48752473f3 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -846,7 +846,7 @@ describe API::API, api: true do end end - describe 'PUT /user/:id/block' do + describe 'PUT /users/:id/block' do before { admin } it 'blocks existing user' do put api("/users/#{user.id}/block", admin) @@ -873,7 +873,7 @@ describe API::API, api: true do end end - describe 'PUT /user/:id/unblock' do + describe 'PUT /users/:id/unblock' do let(:blocked_user) { create(:user, state: 'blocked') } before { admin } @@ -914,7 +914,7 @@ describe API::API, api: true do end end - describe 'GET /user/:id/events' do + describe 'GET /users/:id/events' do let(:user) { create(:user) } let(:project) { create(:empty_project) } let(:note) { create(:note_on_issue, note: 'What an awesome day!', project: project) } -- cgit v1.2.1 From 97731760d7252acf8ee94c707c0e107492b1ef24 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Fri, 21 Oct 2016 18:13:41 +0200 Subject: Re-organize queues to use for Sidekiq Dumping too many jobs in the same queue (e.g. the "default" queue) is a dangerous setup. Jobs that take a long time to process can effectively block any other work from being performed given there are enough of these jobs. Furthermore it becomes harder to monitor the jobs as a single queue could contain jobs for different workers. In such a setup the only reliable way of getting counts per job is to iterate over all jobs in a queue, which is a rather time consuming process. By using separate queues for various workers we have better control over throughput, we can add weight to queues, and we can monitor queues better. Some workers still use the same queue whenever their work is related. For example, the various CI pipeline workers use the same "pipeline" queue. This commit includes a Rails migration that moves Sidekiq jobs from the old queues to the new ones. This migration also takes care of doing the inverse if ever needed. This does require downtime as otherwise new jobs could be scheduled in the old queues after this migration completes. This commit also includes an RSpec test that blacklists the use of the "default" queue and ensures cron workers use the "cronjob" queue. Fixes gitlab-org/gitlab-ce#23370 --- CHANGELOG.md | 1 + app/workers/admin_email_worker.rb | 3 +- app/workers/build_coverage_worker.rb | 2 +- app/workers/build_email_worker.rb | 1 + app/workers/build_finished_worker.rb | 1 + app/workers/build_hooks_worker.rb | 2 +- app/workers/build_success_worker.rb | 2 +- app/workers/clear_database_cache_worker.rb | 1 + app/workers/concerns/build_queue.rb | 8 ++ app/workers/concerns/cronjob_queue.rb | 9 ++ app/workers/concerns/dedicated_sidekiq_queue.rb | 9 ++ app/workers/concerns/pipeline_queue.rb | 8 ++ app/workers/concerns/repository_check_queue.rb | 8 ++ app/workers/delete_user_worker.rb | 1 + app/workers/email_receiver_worker.rb | 3 +- app/workers/emails_on_push_worker.rb | 2 +- app/workers/expire_build_artifacts_worker.rb | 1 + .../expire_build_instance_artifacts_worker.rb | 1 + app/workers/git_garbage_collect_worker.rb | 3 +- app/workers/gitlab_shell_worker.rb | 3 +- app/workers/group_destroy_worker.rb | 3 +- .../import_export_project_cleanup_worker.rb | 3 +- app/workers/irker_worker.rb | 1 + app/workers/merge_worker.rb | 3 +- app/workers/new_note_worker.rb | 3 +- app/workers/pipeline_hooks_worker.rb | 2 +- app/workers/pipeline_metrics_worker.rb | 3 +- app/workers/pipeline_process_worker.rb | 3 +- app/workers/pipeline_success_worker.rb | 2 +- app/workers/pipeline_update_worker.rb | 3 +- app/workers/post_receive.rb | 3 +- app/workers/project_cache_worker.rb | 3 +- app/workers/project_destroy_worker.rb | 3 +- app/workers/project_export_worker.rb | 3 +- app/workers/project_service_worker.rb | 3 +- app/workers/project_web_hook_worker.rb | 3 +- app/workers/prune_old_events_worker.rb | 1 + app/workers/remove_expired_group_links_worker.rb | 1 + app/workers/remove_expired_members_worker.rb | 1 + app/workers/repository_archive_cache_worker.rb | 3 +- app/workers/repository_check/batch_worker.rb | 21 ++-- app/workers/repository_check/clear_worker.rb | 3 +- .../repository_check/single_repository_worker.rb | 3 +- app/workers/repository_fork_worker.rb | 3 +- app/workers/repository_import_worker.rb | 3 +- app/workers/requests_profiles_worker.rb | 3 +- app/workers/stuck_ci_builds_worker.rb | 1 + app/workers/system_hook_worker.rb | 3 +- app/workers/trending_projects_worker.rb | 3 +- app/workers/update_merge_requests_worker.rb | 1 + bin/background_jobs | 3 +- config/application.rb | 3 +- config/sidekiq_queues.yml | 46 +++++++++ ...19190736_migrate_sidekiq_queues_from_default.rb | 109 +++++++++++++++++++++ doc/development/README.md | 3 +- doc/development/sidekiq_style_guide.md | 38 +++++++ spec/workers/concerns/build_queue_spec.rb | 14 +++ spec/workers/concerns/cronjob_queue_spec.rb | 18 ++++ .../concerns/dedicated_sidekiq_queue_spec.rb | 20 ++++ spec/workers/concerns/pipeline_queue_spec.rb | 14 +++ .../concerns/repository_check_queue_spec.rb | 18 ++++ spec/workers/every_sidekiq_worker_spec.rb | 44 +++++++++ 62 files changed, 425 insertions(+), 68 deletions(-) create mode 100644 app/workers/concerns/build_queue.rb create mode 100644 app/workers/concerns/cronjob_queue.rb create mode 100644 app/workers/concerns/dedicated_sidekiq_queue.rb create mode 100644 app/workers/concerns/pipeline_queue.rb create mode 100644 app/workers/concerns/repository_check_queue.rb create mode 100644 config/sidekiq_queues.yml create mode 100644 db/migrate/20161019190736_migrate_sidekiq_queues_from_default.rb create mode 100644 doc/development/sidekiq_style_guide.md create mode 100644 spec/workers/concerns/build_queue_spec.rb create mode 100644 spec/workers/concerns/cronjob_queue_spec.rb create mode 100644 spec/workers/concerns/dedicated_sidekiq_queue_spec.rb create mode 100644 spec/workers/concerns/pipeline_queue_spec.rb create mode 100644 spec/workers/concerns/repository_check_queue_spec.rb create mode 100644 spec/workers/every_sidekiq_worker_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 518d0362d07..52d435df8f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Please view this file on the master branch, on stable branches it's out of date. ## 8.13.0 (2016-10-22) - Fix save button on project pipeline settings page. (!6955) + - All Sidekiq workers now use their own queue - Avoid race condition when asynchronously removing expired artifacts. (!6881) - Improve Merge When Build Succeeds triggers and execute on pipeline success. (!6675) - Respond with 404 Not Found for non-existent tags (Linus Thiel) diff --git a/app/workers/admin_email_worker.rb b/app/workers/admin_email_worker.rb index 667fff031dd..c2dc955b27c 100644 --- a/app/workers/admin_email_worker.rb +++ b/app/workers/admin_email_worker.rb @@ -1,7 +1,6 @@ class AdminEmailWorker include Sidekiq::Worker - - sidekiq_options retry: false # this job auto-repeats via sidekiq-cron + include CronjobQueue def perform repository_check_failed_count = Project.where(last_repository_check_failed: true).count diff --git a/app/workers/build_coverage_worker.rb b/app/workers/build_coverage_worker.rb index 0680645a8db..def0ab1dde1 100644 --- a/app/workers/build_coverage_worker.rb +++ b/app/workers/build_coverage_worker.rb @@ -1,6 +1,6 @@ class BuildCoverageWorker include Sidekiq::Worker - sidekiq_options queue: :default + include BuildQueue def perform(build_id) Ci::Build.find_by(id: build_id) diff --git a/app/workers/build_email_worker.rb b/app/workers/build_email_worker.rb index 1c7a04a66a8..5fdb1f2baa0 100644 --- a/app/workers/build_email_worker.rb +++ b/app/workers/build_email_worker.rb @@ -1,5 +1,6 @@ class BuildEmailWorker include Sidekiq::Worker + include BuildQueue def perform(build_id, recipients, push_data) recipients.each do |recipient| diff --git a/app/workers/build_finished_worker.rb b/app/workers/build_finished_worker.rb index e7286b77ac5..466410bf08c 100644 --- a/app/workers/build_finished_worker.rb +++ b/app/workers/build_finished_worker.rb @@ -1,5 +1,6 @@ class BuildFinishedWorker include Sidekiq::Worker + include BuildQueue def perform(build_id) Ci::Build.find_by(id: build_id).try do |build| diff --git a/app/workers/build_hooks_worker.rb b/app/workers/build_hooks_worker.rb index e22ececb3fd..9965af935d4 100644 --- a/app/workers/build_hooks_worker.rb +++ b/app/workers/build_hooks_worker.rb @@ -1,6 +1,6 @@ class BuildHooksWorker include Sidekiq::Worker - sidekiq_options queue: :default + include BuildQueue def perform(build_id) Ci::Build.find_by(id: build_id) diff --git a/app/workers/build_success_worker.rb b/app/workers/build_success_worker.rb index 500d357ce31..e0ad5268664 100644 --- a/app/workers/build_success_worker.rb +++ b/app/workers/build_success_worker.rb @@ -1,6 +1,6 @@ class BuildSuccessWorker include Sidekiq::Worker - sidekiq_options queue: :default + include BuildQueue def perform(build_id) Ci::Build.find_by(id: build_id).try do |build| diff --git a/app/workers/clear_database_cache_worker.rb b/app/workers/clear_database_cache_worker.rb index c541daba50e..c4cb4733482 100644 --- a/app/workers/clear_database_cache_worker.rb +++ b/app/workers/clear_database_cache_worker.rb @@ -1,6 +1,7 @@ # This worker clears all cache fields in the database, working in batches. class ClearDatabaseCacheWorker include Sidekiq::Worker + include DedicatedSidekiqQueue BATCH_SIZE = 1000 diff --git a/app/workers/concerns/build_queue.rb b/app/workers/concerns/build_queue.rb new file mode 100644 index 00000000000..cf0ead40a8b --- /dev/null +++ b/app/workers/concerns/build_queue.rb @@ -0,0 +1,8 @@ +# Concern for setting Sidekiq settings for the various CI build workers. +module BuildQueue + extend ActiveSupport::Concern + + included do + sidekiq_options queue: :build + end +end diff --git a/app/workers/concerns/cronjob_queue.rb b/app/workers/concerns/cronjob_queue.rb new file mode 100644 index 00000000000..e918bb011e0 --- /dev/null +++ b/app/workers/concerns/cronjob_queue.rb @@ -0,0 +1,9 @@ +# Concern that sets various Sidekiq settings for workers executed using a +# cronjob. +module CronjobQueue + extend ActiveSupport::Concern + + included do + sidekiq_options queue: :cronjob, retry: false + end +end diff --git a/app/workers/concerns/dedicated_sidekiq_queue.rb b/app/workers/concerns/dedicated_sidekiq_queue.rb new file mode 100644 index 00000000000..132bae6022b --- /dev/null +++ b/app/workers/concerns/dedicated_sidekiq_queue.rb @@ -0,0 +1,9 @@ +# Concern that sets the queue of a Sidekiq worker based on the worker's class +# name/namespace. +module DedicatedSidekiqQueue + extend ActiveSupport::Concern + + included do + sidekiq_options queue: name.sub(/Worker\z/, '').underscore.tr('/', '_') + end +end diff --git a/app/workers/concerns/pipeline_queue.rb b/app/workers/concerns/pipeline_queue.rb new file mode 100644 index 00000000000..ca3860e1d38 --- /dev/null +++ b/app/workers/concerns/pipeline_queue.rb @@ -0,0 +1,8 @@ +# Concern for setting Sidekiq settings for the various CI pipeline workers. +module PipelineQueue + extend ActiveSupport::Concern + + included do + sidekiq_options queue: :pipeline + end +end diff --git a/app/workers/concerns/repository_check_queue.rb b/app/workers/concerns/repository_check_queue.rb new file mode 100644 index 00000000000..a597321ccf4 --- /dev/null +++ b/app/workers/concerns/repository_check_queue.rb @@ -0,0 +1,8 @@ +# Concern for setting Sidekiq settings for the various repository check workers. +module RepositoryCheckQueue + extend ActiveSupport::Concern + + included do + sidekiq_options queue: :repository_check, retry: false + end +end diff --git a/app/workers/delete_user_worker.rb b/app/workers/delete_user_worker.rb index 6ff361e4d80..3194c389b3d 100644 --- a/app/workers/delete_user_worker.rb +++ b/app/workers/delete_user_worker.rb @@ -1,5 +1,6 @@ class DeleteUserWorker include Sidekiq::Worker + include DedicatedSidekiqQueue def perform(current_user_id, delete_user_id, options = {}) delete_user = User.find(delete_user_id) diff --git a/app/workers/email_receiver_worker.rb b/app/workers/email_receiver_worker.rb index 842eebdea9e..d3f7e479a8d 100644 --- a/app/workers/email_receiver_worker.rb +++ b/app/workers/email_receiver_worker.rb @@ -1,7 +1,6 @@ class EmailReceiverWorker include Sidekiq::Worker - - sidekiq_options queue: :incoming_email + include DedicatedSidekiqQueue def perform(raw) return unless Gitlab::IncomingEmail.enabled? diff --git a/app/workers/emails_on_push_worker.rb b/app/workers/emails_on_push_worker.rb index 1dc7e0adef7..b9cd49985dc 100644 --- a/app/workers/emails_on_push_worker.rb +++ b/app/workers/emails_on_push_worker.rb @@ -1,7 +1,7 @@ class EmailsOnPushWorker include Sidekiq::Worker + include DedicatedSidekiqQueue - sidekiq_options queue: :mailers attr_reader :email, :skip_premailer def perform(project_id, recipients, push_data, options = {}) diff --git a/app/workers/expire_build_artifacts_worker.rb b/app/workers/expire_build_artifacts_worker.rb index 174eabff9fd..a27585fd389 100644 --- a/app/workers/expire_build_artifacts_worker.rb +++ b/app/workers/expire_build_artifacts_worker.rb @@ -1,5 +1,6 @@ class ExpireBuildArtifactsWorker include Sidekiq::Worker + include CronjobQueue def perform Rails.logger.info 'Scheduling removal of build artifacts' diff --git a/app/workers/expire_build_instance_artifacts_worker.rb b/app/workers/expire_build_instance_artifacts_worker.rb index d9e2cc37bb3..eb403c134d1 100644 --- a/app/workers/expire_build_instance_artifacts_worker.rb +++ b/app/workers/expire_build_instance_artifacts_worker.rb @@ -1,5 +1,6 @@ class ExpireBuildInstanceArtifactsWorker include Sidekiq::Worker + include DedicatedSidekiqQueue def perform(build_id) build = Ci::Build diff --git a/app/workers/git_garbage_collect_worker.rb b/app/workers/git_garbage_collect_worker.rb index a6cefd4d601..65f8093b5b0 100644 --- a/app/workers/git_garbage_collect_worker.rb +++ b/app/workers/git_garbage_collect_worker.rb @@ -1,8 +1,9 @@ class GitGarbageCollectWorker include Sidekiq::Worker include Gitlab::ShellAdapter + include DedicatedSidekiqQueue - sidekiq_options queue: :gitlab_shell, retry: false + sidekiq_options retry: false def perform(project_id) project = Project.find(project_id) diff --git a/app/workers/gitlab_shell_worker.rb b/app/workers/gitlab_shell_worker.rb index cfeda88bbc5..964287a1793 100644 --- a/app/workers/gitlab_shell_worker.rb +++ b/app/workers/gitlab_shell_worker.rb @@ -1,8 +1,7 @@ class GitlabShellWorker include Sidekiq::Worker include Gitlab::ShellAdapter - - sidekiq_options queue: :gitlab_shell + include DedicatedSidekiqQueue def perform(action, *arg) gitlab_shell.send(action, *arg) diff --git a/app/workers/group_destroy_worker.rb b/app/workers/group_destroy_worker.rb index 5048746f09b..a49a5fd0855 100644 --- a/app/workers/group_destroy_worker.rb +++ b/app/workers/group_destroy_worker.rb @@ -1,7 +1,6 @@ class GroupDestroyWorker include Sidekiq::Worker - - sidekiq_options queue: :default + include DedicatedSidekiqQueue def perform(group_id, user_id) begin diff --git a/app/workers/import_export_project_cleanup_worker.rb b/app/workers/import_export_project_cleanup_worker.rb index 72e3a9ae734..7957ed807ab 100644 --- a/app/workers/import_export_project_cleanup_worker.rb +++ b/app/workers/import_export_project_cleanup_worker.rb @@ -1,7 +1,6 @@ class ImportExportProjectCleanupWorker include Sidekiq::Worker - - sidekiq_options queue: :default + include CronjobQueue def perform ImportExportCleanUpService.new.execute diff --git a/app/workers/irker_worker.rb b/app/workers/irker_worker.rb index 19f38358eb5..7e44b241743 100644 --- a/app/workers/irker_worker.rb +++ b/app/workers/irker_worker.rb @@ -3,6 +3,7 @@ require 'socket' class IrkerWorker include Sidekiq::Worker + include DedicatedSidekiqQueue def perform(project_id, chans, colors, push_data, settings) project = Project.find(project_id) diff --git a/app/workers/merge_worker.rb b/app/workers/merge_worker.rb index c87c0a252b1..79efca4f2f9 100644 --- a/app/workers/merge_worker.rb +++ b/app/workers/merge_worker.rb @@ -1,7 +1,6 @@ class MergeWorker include Sidekiq::Worker - - sidekiq_options queue: :default + include DedicatedSidekiqQueue def perform(merge_request_id, current_user_id, params) params = params.with_indifferent_access diff --git a/app/workers/new_note_worker.rb b/app/workers/new_note_worker.rb index 1b3232cd365..c3e62bb88c0 100644 --- a/app/workers/new_note_worker.rb +++ b/app/workers/new_note_worker.rb @@ -1,7 +1,6 @@ class NewNoteWorker include Sidekiq::Worker - - sidekiq_options queue: :default + include DedicatedSidekiqQueue def perform(note_id, note_params) note = Note.find(note_id) diff --git a/app/workers/pipeline_hooks_worker.rb b/app/workers/pipeline_hooks_worker.rb index ab5e9f6daad..7e36eacebf8 100644 --- a/app/workers/pipeline_hooks_worker.rb +++ b/app/workers/pipeline_hooks_worker.rb @@ -1,6 +1,6 @@ class PipelineHooksWorker include Sidekiq::Worker - sidekiq_options queue: :default + include PipelineQueue def perform(pipeline_id) Ci::Pipeline.find_by(id: pipeline_id) diff --git a/app/workers/pipeline_metrics_worker.rb b/app/workers/pipeline_metrics_worker.rb index 7bb92df3bbd..34f6ef161fb 100644 --- a/app/workers/pipeline_metrics_worker.rb +++ b/app/workers/pipeline_metrics_worker.rb @@ -1,7 +1,6 @@ class PipelineMetricsWorker include Sidekiq::Worker - - sidekiq_options queue: :default + include PipelineQueue def perform(pipeline_id) Ci::Pipeline.find_by(id: pipeline_id).try do |pipeline| diff --git a/app/workers/pipeline_process_worker.rb b/app/workers/pipeline_process_worker.rb index f44227d7086..357e4a9a1c3 100644 --- a/app/workers/pipeline_process_worker.rb +++ b/app/workers/pipeline_process_worker.rb @@ -1,7 +1,6 @@ class PipelineProcessWorker include Sidekiq::Worker - - sidekiq_options queue: :default + include PipelineQueue def perform(pipeline_id) Ci::Pipeline.find_by(id: pipeline_id) diff --git a/app/workers/pipeline_success_worker.rb b/app/workers/pipeline_success_worker.rb index 5dd443fea59..2aa6fff24da 100644 --- a/app/workers/pipeline_success_worker.rb +++ b/app/workers/pipeline_success_worker.rb @@ -1,6 +1,6 @@ class PipelineSuccessWorker include Sidekiq::Worker - sidekiq_options queue: :default + include PipelineQueue def perform(pipeline_id) Ci::Pipeline.find_by(id: pipeline_id).try do |pipeline| diff --git a/app/workers/pipeline_update_worker.rb b/app/workers/pipeline_update_worker.rb index 44a7f24e401..96c4152c674 100644 --- a/app/workers/pipeline_update_worker.rb +++ b/app/workers/pipeline_update_worker.rb @@ -1,7 +1,6 @@ class PipelineUpdateWorker include Sidekiq::Worker - - sidekiq_options queue: :default + include PipelineQueue def perform(pipeline_id) Ci::Pipeline.find_by(id: pipeline_id) diff --git a/app/workers/post_receive.rb b/app/workers/post_receive.rb index a9a2b716005..eee0ca12af9 100644 --- a/app/workers/post_receive.rb +++ b/app/workers/post_receive.rb @@ -1,7 +1,6 @@ class PostReceive include Sidekiq::Worker - - sidekiq_options queue: :post_receive + include DedicatedSidekiqQueue def perform(repo_path, identifier, changes) if path = Gitlab.config.repositories.storages.find { |p| repo_path.start_with?(p[1].to_s) } diff --git a/app/workers/project_cache_worker.rb b/app/workers/project_cache_worker.rb index 0d524e88dc3..71b274e0c99 100644 --- a/app/workers/project_cache_worker.rb +++ b/app/workers/project_cache_worker.rb @@ -5,8 +5,7 @@ # storage engine as much. class ProjectCacheWorker include Sidekiq::Worker - - sidekiq_options queue: :default + include DedicatedSidekiqQueue LEASE_TIMEOUT = 15.minutes.to_i diff --git a/app/workers/project_destroy_worker.rb b/app/workers/project_destroy_worker.rb index 3062301a9b1..b462327490e 100644 --- a/app/workers/project_destroy_worker.rb +++ b/app/workers/project_destroy_worker.rb @@ -1,7 +1,6 @@ class ProjectDestroyWorker include Sidekiq::Worker - - sidekiq_options queue: :default + include DedicatedSidekiqQueue def perform(project_id, user_id, params) begin diff --git a/app/workers/project_export_worker.rb b/app/workers/project_export_worker.rb index 615311e63f5..6009aa1b191 100644 --- a/app/workers/project_export_worker.rb +++ b/app/workers/project_export_worker.rb @@ -1,7 +1,8 @@ class ProjectExportWorker include Sidekiq::Worker + include DedicatedSidekiqQueue - sidekiq_options queue: :gitlab_shell, retry: 3 + sidekiq_options retry: 3 def perform(current_user_id, project_id) current_user = User.find(current_user_id) diff --git a/app/workers/project_service_worker.rb b/app/workers/project_service_worker.rb index 64d39c4d3f7..fdfdeab7b41 100644 --- a/app/workers/project_service_worker.rb +++ b/app/workers/project_service_worker.rb @@ -1,7 +1,6 @@ class ProjectServiceWorker include Sidekiq::Worker - - sidekiq_options queue: :project_web_hook + include DedicatedSidekiqQueue def perform(hook_id, data) data = data.with_indifferent_access diff --git a/app/workers/project_web_hook_worker.rb b/app/workers/project_web_hook_worker.rb index fb878965288..efb85eafd15 100644 --- a/app/workers/project_web_hook_worker.rb +++ b/app/workers/project_web_hook_worker.rb @@ -1,7 +1,6 @@ class ProjectWebHookWorker include Sidekiq::Worker - - sidekiq_options queue: :project_web_hook + include DedicatedSidekiqQueue def perform(hook_id, data, hook_name) data = data.with_indifferent_access diff --git a/app/workers/prune_old_events_worker.rb b/app/workers/prune_old_events_worker.rb index 5883cafe1d1..392abb9c21b 100644 --- a/app/workers/prune_old_events_worker.rb +++ b/app/workers/prune_old_events_worker.rb @@ -1,5 +1,6 @@ class PruneOldEventsWorker include Sidekiq::Worker + include CronjobQueue def perform # Contribution calendar shows maximum 12 months of events. diff --git a/app/workers/remove_expired_group_links_worker.rb b/app/workers/remove_expired_group_links_worker.rb index 246c8b6650a..2a619f83410 100644 --- a/app/workers/remove_expired_group_links_worker.rb +++ b/app/workers/remove_expired_group_links_worker.rb @@ -1,5 +1,6 @@ class RemoveExpiredGroupLinksWorker include Sidekiq::Worker + include CronjobQueue def perform ProjectGroupLink.expired.destroy_all diff --git a/app/workers/remove_expired_members_worker.rb b/app/workers/remove_expired_members_worker.rb index cf765af97ce..31f652e5f9b 100644 --- a/app/workers/remove_expired_members_worker.rb +++ b/app/workers/remove_expired_members_worker.rb @@ -1,5 +1,6 @@ class RemoveExpiredMembersWorker include Sidekiq::Worker + include CronjobQueue def perform Member.expired.find_each do |member| diff --git a/app/workers/repository_archive_cache_worker.rb b/app/workers/repository_archive_cache_worker.rb index a2e49c61f59..e47069df189 100644 --- a/app/workers/repository_archive_cache_worker.rb +++ b/app/workers/repository_archive_cache_worker.rb @@ -1,7 +1,6 @@ class RepositoryArchiveCacheWorker include Sidekiq::Worker - - sidekiq_options queue: :default + include CronjobQueue def perform RepositoryArchiveCleanUpService.new.execute diff --git a/app/workers/repository_check/batch_worker.rb b/app/workers/repository_check/batch_worker.rb index a3e16fa5212..c3e7491ec4e 100644 --- a/app/workers/repository_check/batch_worker.rb +++ b/app/workers/repository_check/batch_worker.rb @@ -1,14 +1,13 @@ module RepositoryCheck class BatchWorker include Sidekiq::Worker - + include CronjobQueue + RUN_TIME = 3600 - - sidekiq_options retry: false - + def perform start = Time.now - + # This loop will break after a little more than one hour ('a little # more' because `git fsck` may take a few minutes), or if it runs out of # projects to check. By default sidekiq-cron will start a new @@ -17,15 +16,15 @@ module RepositoryCheck project_ids.each do |project_id| break if Time.now - start >= RUN_TIME break unless current_settings.repository_checks_enabled - + next unless try_obtain_lease(project_id) - + SingleRepositoryWorker.new.perform(project_id) end end - + private - + # Project.find_each does not support WHERE clauses and # Project.find_in_batches does not support ordering. So we just build an # array of ID's. This is OK because we do it only once an hour, because @@ -39,7 +38,7 @@ module RepositoryCheck reorder('last_repository_check_at ASC').limit(limit).pluck(:id) never_checked_projects + old_check_projects end - + def try_obtain_lease(id) # Use a 24-hour timeout because on servers/projects where 'git fsck' is # super slow we definitely do not want to run it twice in parallel. @@ -48,7 +47,7 @@ module RepositoryCheck timeout: 24.hours ).try_obtain end - + def current_settings # No caching of the settings! If we cache them and an admin disables # this feature, an active RepositoryCheckWorker would keep going for up diff --git a/app/workers/repository_check/clear_worker.rb b/app/workers/repository_check/clear_worker.rb index b7202ddff34..1f1b38540ee 100644 --- a/app/workers/repository_check/clear_worker.rb +++ b/app/workers/repository_check/clear_worker.rb @@ -1,8 +1,7 @@ module RepositoryCheck class ClearWorker include Sidekiq::Worker - - sidekiq_options retry: false + include RepositoryCheckQueue def perform # Do small batched updates because these updates will be slow and locking diff --git a/app/workers/repository_check/single_repository_worker.rb b/app/workers/repository_check/single_repository_worker.rb index 98ddf5d0688..3d8bfc6fc6c 100644 --- a/app/workers/repository_check/single_repository_worker.rb +++ b/app/workers/repository_check/single_repository_worker.rb @@ -1,8 +1,7 @@ module RepositoryCheck class SingleRepositoryWorker include Sidekiq::Worker - - sidekiq_options retry: false + include RepositoryCheckQueue def perform(project_id) project = Project.find(project_id) diff --git a/app/workers/repository_fork_worker.rb b/app/workers/repository_fork_worker.rb index 61ed1c38ac4..efc99ec962a 100644 --- a/app/workers/repository_fork_worker.rb +++ b/app/workers/repository_fork_worker.rb @@ -1,8 +1,7 @@ class RepositoryForkWorker include Sidekiq::Worker include Gitlab::ShellAdapter - - sidekiq_options queue: :gitlab_shell + include DedicatedSidekiqQueue def perform(project_id, forked_from_repository_storage_path, source_path, target_path) Gitlab::Metrics.add_event(:fork_repository, diff --git a/app/workers/repository_import_worker.rb b/app/workers/repository_import_worker.rb index d2ca8813ab9..c8a77e21c12 100644 --- a/app/workers/repository_import_worker.rb +++ b/app/workers/repository_import_worker.rb @@ -1,8 +1,7 @@ class RepositoryImportWorker include Sidekiq::Worker include Gitlab::ShellAdapter - - sidekiq_options queue: :gitlab_shell + include DedicatedSidekiqQueue attr_accessor :project, :current_user diff --git a/app/workers/requests_profiles_worker.rb b/app/workers/requests_profiles_worker.rb index 9dd228a2483..703b025d76e 100644 --- a/app/workers/requests_profiles_worker.rb +++ b/app/workers/requests_profiles_worker.rb @@ -1,7 +1,6 @@ class RequestsProfilesWorker include Sidekiq::Worker - - sidekiq_options queue: :default + include CronjobQueue def perform Gitlab::RequestProfiler.remove_all_profiles diff --git a/app/workers/stuck_ci_builds_worker.rb b/app/workers/stuck_ci_builds_worker.rb index 6828013b377..b70df5a1afa 100644 --- a/app/workers/stuck_ci_builds_worker.rb +++ b/app/workers/stuck_ci_builds_worker.rb @@ -1,5 +1,6 @@ class StuckCiBuildsWorker include Sidekiq::Worker + include CronjobQueue BUILD_STUCK_TIMEOUT = 1.day diff --git a/app/workers/system_hook_worker.rb b/app/workers/system_hook_worker.rb index a122c274763..baf2f12eeac 100644 --- a/app/workers/system_hook_worker.rb +++ b/app/workers/system_hook_worker.rb @@ -1,7 +1,6 @@ class SystemHookWorker include Sidekiq::Worker - - sidekiq_options queue: :system_hook + include DedicatedSidekiqQueue def perform(hook_id, data, hook_name) SystemHook.find(hook_id).execute(data, hook_name) diff --git a/app/workers/trending_projects_worker.rb b/app/workers/trending_projects_worker.rb index df4c4a6628b..0531630d13a 100644 --- a/app/workers/trending_projects_worker.rb +++ b/app/workers/trending_projects_worker.rb @@ -1,7 +1,6 @@ class TrendingProjectsWorker include Sidekiq::Worker - - sidekiq_options queue: :trending_projects + include CronjobQueue def perform Rails.logger.info('Refreshing trending projects') diff --git a/app/workers/update_merge_requests_worker.rb b/app/workers/update_merge_requests_worker.rb index 03f0528cdae..acc4d858136 100644 --- a/app/workers/update_merge_requests_worker.rb +++ b/app/workers/update_merge_requests_worker.rb @@ -1,5 +1,6 @@ class UpdateMergeRequestsWorker include Sidekiq::Worker + include DedicatedSidekiqQueue def perform(project_id, user_id, oldrev, newrev, ref) project = Project.find_by(id: project_id) diff --git a/bin/background_jobs b/bin/background_jobs index 25a578a1c49..f28e2f722dc 100755 --- a/bin/background_jobs +++ b/bin/background_jobs @@ -4,6 +4,7 @@ cd $(dirname $0)/.. app_root=$(pwd) sidekiq_pidfile="$app_root/tmp/pids/sidekiq.pid" sidekiq_logfile="$app_root/log/sidekiq.log" +sidekiq_config="$app_root/config/sidekiq_queues.yml" gitlab_user=$(ls -l config.ru | awk '{print $3}') warn() @@ -37,7 +38,7 @@ start_no_deamonize() start_sidekiq() { - exec bundle exec sidekiq -q post_receive -q mailers -q archive_repo -q system_hook -q project_web_hook -q gitlab_shell -q incoming_email -q runner -q common -q default -e $RAILS_ENV -P $sidekiq_pidfile "$@" + exec bundle exec sidekiq -C "${sidekiq_config}" -e $RAILS_ENV -P $sidekiq_pidfile "$@" } load_ok() diff --git a/config/application.rb b/config/application.rb index f3337b00dc6..92c8467e7f4 100644 --- a/config/application.rb +++ b/config/application.rb @@ -24,7 +24,8 @@ module Gitlab #{config.root}/app/models/ci #{config.root}/app/models/hooks #{config.root}/app/models/members - #{config.root}/app/models/project_services)) + #{config.root}/app/models/project_services + #{config.root}/app/workers/concerns)) config.generators.templates.push("#{config.root}/generator_templates") diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml new file mode 100644 index 00000000000..c2e880e891f --- /dev/null +++ b/config/sidekiq_queues.yml @@ -0,0 +1,46 @@ +# This configuration file should be exclusively used to set queue settings for +# Sidekiq. Any other setting should be specified using the Sidekiq CLI or the +# Sidekiq Ruby API (see config/initializers/sidekiq.rb). +--- +# All the queues to process and their weights. Every queue _must_ have a weight +# defined. +# +# The available weights are as follows +# +# 1: low priority +# 2: medium priority +# 3: high priority +# 5: _super_ high priority, this should only be used for _very_ important queues +# +# As per http://stackoverflow.com/a/21241357/290102 the formula for calculating +# the likelihood of a job being popped off a queue (given all queues have work +# to perform) is: +# +# chance = (queue weight / total weight of all queues) * 100 +:queues: + - [post_receive, 5] + - [merge, 5] + - [update_merge_requests, 3] + - [new_note, 2] + - [build, 2] + - [pipeline, 2] + - [gitlab_shell, 2] + - [email_receiver, 2] + - [emails_on_push, 2] + - [repository_fork, 1] + - [repository_import, 1] + - [project_service, 1] + - [clear_database_cache, 1] + - [delete_user, 1] + - [expire_build_instance_artifacts, 1] + - [group_destroy, 1] + - [irker, 1] + - [project_cache, 1] + - [project_destroy, 1] + - [project_export, 1] + - [project_web_hook, 1] + - [repository_check, 1] + - [system_hook, 1] + - [git_garbage_collect, 1] + - [cronjob, 1] + - [default, 1] diff --git a/db/migrate/20161019190736_migrate_sidekiq_queues_from_default.rb b/db/migrate/20161019190736_migrate_sidekiq_queues_from_default.rb new file mode 100644 index 00000000000..e875213ab96 --- /dev/null +++ b/db/migrate/20161019190736_migrate_sidekiq_queues_from_default.rb @@ -0,0 +1,109 @@ +require 'json' + +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class MigrateSidekiqQueuesFromDefault < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = true + + DOWNTIME_REASON = <<-EOF + Moving Sidekiq jobs from queues requires Sidekiq to be stopped. Not stopping + Sidekiq will result in the loss of jobs that are scheduled after this + migration completes. + EOF + + disable_ddl_transaction! + + # Jobs for which the queue names have been changed (e.g. multiple workers + # using the same non-default queue). + # + # The keys are the old queue names, the values the jobs to move and their new + # queue names. + RENAMED_QUEUES = { + gitlab_shell: { + 'GitGarbageCollectorWorker' => :git_garbage_collector, + 'ProjectExportWorker' => :project_export, + 'RepositoryForkWorker' => :repository_fork, + 'RepositoryImportWorker' => :repository_import + }, + project_web_hook: { + 'ProjectServiceWorker' => :project_service + }, + incoming_email: { + 'EmailReceiverWorker' => :email_receiver + }, + mailers: { + 'EmailsOnPushWorker' => :emails_on_push + }, + default: { + 'AdminEmailWorker' => :cronjob, + 'BuildCoverageWorker' => :build, + 'BuildEmailWorker' => :build, + 'BuildFinishedWorker' => :build, + 'BuildHooksWorker' => :build, + 'BuildSuccessWorker' => :build, + 'ClearDatabaseCacheWorker' => :clear_database_cache, + 'DeleteUserWorker' => :delete_user, + 'ExpireBuildArtifactsWorker' => :cronjob, + 'ExpireBuildInstanceArtifactsWorker' => :expire_build_instance_artifacts, + 'GroupDestroyWorker' => :group_destroy, + 'ImportExportProjectCleanupWorker' => :cronjob, + 'IrkerWorker' => :irker, + 'MergeWorker' => :merge, + 'NewNoteWorker' => :new_note, + 'PipelineHooksWorker' => :pipeline, + 'PipelineMetricsWorker' => :pipeline, + 'PipelineProcessWorker' => :pipeline, + 'PipelineSuccessWorker' => :pipeline, + 'PipelineUpdateWorker' => :pipeline, + 'ProjectCacheWorker' => :project_cache, + 'ProjectDestroyWorker' => :project_destroy, + 'PruneOldEventsWorker' => :cronjob, + 'RemoveExpiredGroupLinksWorker' => :cronjob, + 'RemoveExpiredMembersWorker' => :cronjob, + 'RepositoryArchiveCacheWorker' => :cronjob, + 'RepositoryCheck::BatchWorker' => :cronjob, + 'RepositoryCheck::ClearWorker' => :repository_check, + 'RepositoryCheck::SingleRepositoryWorker' => :repository_check, + 'RequestsProfilesWorker' => :cronjob, + 'StuckCiBuildsWorker' => :cronjob, + 'UpdateMergeRequestsWorker' => :update_merge_requests + } + } + + def up + Sidekiq.redis do |redis| + RENAMED_QUEUES.each do |queue, jobs| + migrate_from_queue(redis, queue, jobs) + end + end + end + + def down + Sidekiq.redis do |redis| + RENAMED_QUEUES.each do |dest_queue, jobs| + jobs.each do |worker, from_queue| + migrate_from_queue(redis, from_queue, worker => dest_queue) + end + end + end + end + + def migrate_from_queue(redis, queue, job_mapping) + while job = redis.lpop("queue:#{queue}") + payload = JSON.load(job) + new_queue = job_mapping[payload['class']] + + # If we have no target queue to migrate to we're probably dealing with + # some ancient job for which the worker no longer exists. In that case + # there's no sane option we can take, other than just dropping the job. + next unless new_queue + + payload['queue'] = new_queue + + redis.lpush("queue:#{new_queue}", JSON.dump(payload)) + end + end +end diff --git a/doc/development/README.md b/doc/development/README.md index 9706cb1de7f..fb6a8a5b095 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -14,7 +14,8 @@ - [Testing standards and style guidelines](testing.md) - [UI guide](ui_guide.md) for building GitLab with existing CSS styles and elements - [Frontend guidelines](frontend.md) -- [SQL guidelines](sql.md) for SQL guidelines +- [SQL guidelines](sql.md) for working with SQL queries +- [Sidekiq guidelines](sidekiq_style_guide.md) for working with Sidekiq workers ## Process diff --git a/doc/development/sidekiq_style_guide.md b/doc/development/sidekiq_style_guide.md new file mode 100644 index 00000000000..e3a20f29a09 --- /dev/null +++ b/doc/development/sidekiq_style_guide.md @@ -0,0 +1,38 @@ +# Sidekiq Style Guide + +This document outlines various guidelines that should be followed when adding or +modifying Sidekiq workers. + +## Default Queue + +Use of the "default" queue is not allowed. Every worker should use a queue that +matches the worker's purpose the closest. For example, workers that are to be +executed periodically should use the "cronjob" queue. + +A list of all available queues can be found in `config/sidekiq_queues.yml`. + +## Dedicated Queues + +Most workers should use their own queue. To ease this process a worker can +include the `DedicatedSidekiqQueue` concern as follows: + +```ruby +class ProcessSomethingWorker + include Sidekiq::Worker + include DedicatedSidekiqQueue +end +``` + +This will set the queue name based on the class' name, minus the `Worker` +suffix. In the above example this would lead to the queue being +`process_something`. + +In some cases multiple workers do use the same queue. For example, the various +workers for updating CI pipelines all use the `pipeline` queue. Adding workers +to existing queues should be done with care, as adding more workers can lead to +slow jobs blocking work (even for different jobs) on the shared queue. + +## Tests + +Each Sidekiq worker must be tested using RSpec, just like any other class. These +tests should be placed in `spec/workers`. diff --git a/spec/workers/concerns/build_queue_spec.rb b/spec/workers/concerns/build_queue_spec.rb new file mode 100644 index 00000000000..6bf955e0be2 --- /dev/null +++ b/spec/workers/concerns/build_queue_spec.rb @@ -0,0 +1,14 @@ +require 'spec_helper' + +describe BuildQueue do + let(:worker) do + Class.new do + include Sidekiq::Worker + include BuildQueue + end + end + + it 'sets the queue name of a worker' do + expect(worker.sidekiq_options['queue'].to_s).to eq('build') + end +end diff --git a/spec/workers/concerns/cronjob_queue_spec.rb b/spec/workers/concerns/cronjob_queue_spec.rb new file mode 100644 index 00000000000..5d1336c21a6 --- /dev/null +++ b/spec/workers/concerns/cronjob_queue_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe CronjobQueue do + let(:worker) do + Class.new do + include Sidekiq::Worker + include CronjobQueue + end + end + + it 'sets the queue name of a worker' do + expect(worker.sidekiq_options['queue'].to_s).to eq('cronjob') + end + + it 'disables retrying of failed jobs' do + expect(worker.sidekiq_options['retry']).to eq(false) + end +end diff --git a/spec/workers/concerns/dedicated_sidekiq_queue_spec.rb b/spec/workers/concerns/dedicated_sidekiq_queue_spec.rb new file mode 100644 index 00000000000..512baec8b7e --- /dev/null +++ b/spec/workers/concerns/dedicated_sidekiq_queue_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe DedicatedSidekiqQueue do + let(:worker) do + Class.new do + def self.name + 'Foo::Bar::DummyWorker' + end + + include Sidekiq::Worker + include DedicatedSidekiqQueue + end + end + + describe 'queue names' do + it 'sets the queue name based on the class name' do + expect(worker.sidekiq_options['queue']).to eq('foo_bar_dummy') + end + end +end diff --git a/spec/workers/concerns/pipeline_queue_spec.rb b/spec/workers/concerns/pipeline_queue_spec.rb new file mode 100644 index 00000000000..40794d0e42a --- /dev/null +++ b/spec/workers/concerns/pipeline_queue_spec.rb @@ -0,0 +1,14 @@ +require 'spec_helper' + +describe PipelineQueue do + let(:worker) do + Class.new do + include Sidekiq::Worker + include PipelineQueue + end + end + + it 'sets the queue name of a worker' do + expect(worker.sidekiq_options['queue'].to_s).to eq('pipeline') + end +end diff --git a/spec/workers/concerns/repository_check_queue_spec.rb b/spec/workers/concerns/repository_check_queue_spec.rb new file mode 100644 index 00000000000..8868e969829 --- /dev/null +++ b/spec/workers/concerns/repository_check_queue_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe RepositoryCheckQueue do + let(:worker) do + Class.new do + include Sidekiq::Worker + include RepositoryCheckQueue + end + end + + it 'sets the queue name of a worker' do + expect(worker.sidekiq_options['queue'].to_s).to eq('repository_check') + end + + it 'disables retrying of failed jobs' do + expect(worker.sidekiq_options['retry']).to eq(false) + end +end diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb new file mode 100644 index 00000000000..fc9adf47c1e --- /dev/null +++ b/spec/workers/every_sidekiq_worker_spec.rb @@ -0,0 +1,44 @@ +require 'spec_helper' + +describe 'Every Sidekiq worker' do + let(:workers) do + root = Rails.root.join('app', 'workers') + concerns = root.join('concerns').to_s + + workers = Dir[root.join('**', '*.rb')]. + reject { |path| path.start_with?(concerns) } + + workers.map do |path| + ns = Pathname.new(path).relative_path_from(root).to_s.gsub('.rb', '') + + ns.camelize.constantize + end + end + + it 'does not use the default queue' do + workers.each do |worker| + expect(worker.sidekiq_options['queue'].to_s).not_to eq('default') + end + end + + it 'uses the cronjob queue when the worker runs as a cronjob' do + cron_workers = Settings.cron_jobs. + map { |job_name, options| options['job_class'].constantize }. + to_set + + workers.each do |worker| + next unless cron_workers.include?(worker) + + expect(worker.sidekiq_options['queue'].to_s).to eq('cronjob') + end + end + + it 'defines the queue in the Sidekiq configuration file' do + config = YAML.load_file(Rails.root.join('config', 'sidekiq_queues.yml').to_s) + queue_names = config[:queues].map { |(queue, _)| queue }.to_set + + workers.each do |worker| + expect(queue_names).to include(worker.sidekiq_options['queue'].to_s) + end + end +end -- cgit v1.2.1 From e32858e7734a13b820d2f1439e189c1586af29d8 Mon Sep 17 00:00:00 2001 From: Nur Rony Date: Fri, 21 Oct 2016 23:08:44 +0600 Subject: removes extra line for empty issue description --- app/assets/stylesheets/framework/common.scss | 2 ++ app/views/projects/issues/show.html.haml | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index 81e4e264560..0d00b6a0d9c 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -372,3 +372,5 @@ table { margin-right: -$gl-padding; border-top: 1px solid $border-color; } + +.hide-bottom-border { border-bottom: none;} diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 6f3f238a436..6defd7834bf 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -53,7 +53,7 @@ .issue-details.issuable-details - .detail-page-description.content-block + .detail-page-description.content-block{class: ('hide-bottom-border' unless @issue.description.present? )} %h2.title = markdown_field(@issue, :title) - if @issue.description.present? -- cgit v1.2.1 From 0ca0697a9c2501db23458aa8ef35a8b030625f93 Mon Sep 17 00:00:00 2001 From: Nur Rony Date: Fri, 21 Oct 2016 23:21:31 +0600 Subject: adds entry in CHANGELOG --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 518d0362d07..9207d98327c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Use MergeRequestsClosingIssues cache data on Issue#closed_by_merge_requests method ## 8.13.0 (2016-10-22) - + - Removes extra line for empty issue description. (!7045) - Fix save button on project pipeline settings page. (!6955) - Avoid race condition when asynchronously removing expired artifacts. (!6881) - Improve Merge When Build Succeeds triggers and execute on pipeline success. (!6675) -- cgit v1.2.1 From 3a29ea9da0abd6cfd0788f6d717a08862ed6b062 Mon Sep 17 00:00:00 2001 From: Lemures Lemniscati Date: Sat, 22 Oct 2016 03:07:26 +0900 Subject: Fix documents and comments on Build API `scope`. #23146 #19131 --- CHANGELOG.md | 1 + doc/api/builds.md | 8 ++++---- lib/api/builds.rb | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc3dd569aea..fa9f4dc6091 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Fix: Backup restore doesn't clear cache - Use MergeRequestsClosingIssues cache data on Issue#closed_by_merge_requests method + - Fix documents and comments on Build API `scope` ## 8.13.0 (2016-10-22) diff --git a/doc/api/builds.md b/doc/api/builds.md index e40f198696d..0476cac0eda 100644 --- a/doc/api/builds.md +++ b/doc/api/builds.md @@ -11,10 +11,10 @@ GET /projects/:id/builds | Attribute | Type | Required | Description | |-----------|---------|----------|---------------------| | `id` | integer | yes | The ID of a project | -| `scope` | string **or** array of strings | no | The scope of builds to show, one or array of: `pending`, `running`, `failed`, `success`, `canceled`; showing all builds if none provided | +| `scope` | string **or** array of strings | no | The scope of builds to show, one or array of: `created`, `pending`, `running`, `failed`, `success`, `canceled`, `skipped`; showing all builds if none provided | ``` -curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/builds" +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" 'https://gitlab.example.com/api/v3/projects/1/builds?scope%5B0%5D=pending&scope%5B1%5D=running' ``` Example of response @@ -132,10 +132,10 @@ GET /projects/:id/repository/commits/:sha/builds |-----------|---------|----------|---------------------| | `id` | integer | yes | The ID of a project | | `sha` | string | yes | The SHA id of a commit | -| `scope` | string **or** array of strings | no | The scope of builds to show, one or array of: `pending`, `running`, `failed`, `success`, `canceled`; showing all builds if none provided | +| `scope` | string **or** array of strings | no | The scope of builds to show, one or array of: `created`, `pending`, `running`, `failed`, `success`, `canceled`, `skipped`; showing all builds if none provided | ``` -curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/repository/commits/0ff3ae198f8601a285adcf5c0fff204ee6fba5fd/builds" +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" 'https://gitlab.example.com/api/v3/projects/1/repository/commits/0ff3ae198f8601a285adcf5c0fff204ee6fba5fd/builds?scope%5B0%5D=pending&scope%5B1%5D=running' ``` Example of response diff --git a/lib/api/builds.rb b/lib/api/builds.rb index 52bdbcae5a8..7b00c5037f1 100644 --- a/lib/api/builds.rb +++ b/lib/api/builds.rb @@ -8,7 +8,7 @@ module API # # Parameters: # id (required) - The ID of a project - # scope (optional) - The scope of builds to show (one or array of: pending, running, failed, success, canceled; + # scope (optional) - The scope of builds to show (one or array of: created, pending, running, failed, success, canceled, skipped; # if none provided showing all builds) # Example Request: # GET /projects/:id/builds @@ -25,7 +25,7 @@ module API # Parameters: # id (required) - The ID of a project # sha (required) - The SHA id of a commit - # scope (optional) - The scope of builds to show (one or array of: pending, running, failed, success, canceled; + # scope (optional) - The scope of builds to show (one or array of: created, pending, running, failed, success, canceled, skipped; # if none provided showing all builds) # Example Request: # GET /projects/:id/repository/commits/:sha/builds -- cgit v1.2.1 From 94ceadb4a32a4a5128d43983206518d3159354e0 Mon Sep 17 00:00:00 2001 From: Winnie Date: Fri, 21 Oct 2016 20:18:03 +0000 Subject: Document link syntax introduced by !5586 --- doc/user/markdown.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/doc/user/markdown.md b/doc/user/markdown.md index 56e5b802a52..7a7a0b864bd 100644 --- a/doc/user/markdown.md +++ b/doc/user/markdown.md @@ -501,6 +501,10 @@ There are two ways to create links, inline-style and reference-style. [I'm a reference-style link][Arbitrary case-insensitive reference text] [I'm a relative reference to a repository file](LICENSE) + + [I am an absolute reference within the repository](/doc/user/markdown.md) + + [I link to the Milestones page](/../milestones) [You can use numbers for reference-style link definitions][1] @@ -518,6 +522,10 @@ There are two ways to create links, inline-style and reference-style. [I'm a relative reference to a repository file](LICENSE)[^1] +[I am an absolute reference within the repository](/doc/user/markdown.md) + +[I link to the Milestones page](/../milestones) + [You can use numbers for reference-style link definitions][1] Or leave it empty and use the [link text itself][] -- cgit v1.2.1 From d79c41e7629b6e983c4cf5c0670a1fbab37528ed Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Fri, 21 Oct 2016 22:28:39 -0700 Subject: Fix bug where e-mails were not being sent out via Sidekiq By default, ActionMailer uses the "mailers" queue, but this entry was not included in the list of queues for Sidekiq to use. For more details: * https://github.com/plataformatec/devise/wiki/How-To:-Send-devise-emails-in-background-(Resque,-Sidekiq-and-Delayed::Job) * http://guides.rubyonrails.org/active_job_basics.html --- config/sidekiq_queues.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index c2e880e891f..f36fe893fd0 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -27,6 +27,7 @@ - [gitlab_shell, 2] - [email_receiver, 2] - [emails_on_push, 2] + - [mailers, 2] - [repository_fork, 1] - [repository_import, 1] - [project_service, 1] -- cgit v1.2.1 From 0890aeb61a5378ec3bb98511de236ee01eee8711 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sat, 22 Oct 2016 01:59:40 -0700 Subject: Fix error in generating labels Attempting to generate default set of labels would result in an error: ArgumentError: wrong number of arguments (given 1, expected 0) Closes #23649 --- CHANGELOG.md | 3 +++ lib/gitlab/issues_labels.rb | 2 +- spec/controllers/projects/labels_controller_spec.rb | 15 +++++++++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c39ddca7cf..bfc6a586ade 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ Please view this file on the master branch, on stable branches it's out of date. - Use MergeRequestsClosingIssues cache data on Issue#closed_by_merge_requests method +## 8.13.1 (unreleased) + - Fix error in generating labels + ## 8.13.0 (2016-10-22) - Fix save button on project pipeline settings page. (!6955) diff --git a/lib/gitlab/issues_labels.rb b/lib/gitlab/issues_labels.rb index 01a2c19ab23..dbc759367eb 100644 --- a/lib/gitlab/issues_labels.rb +++ b/lib/gitlab/issues_labels.rb @@ -19,7 +19,7 @@ module Gitlab ] labels.each do |params| - ::Labels::FindOrCreateService.new(project.owner, project).execute(params) + ::Labels::FindOrCreateService.new(project.owner, project, params).execute end end end diff --git a/spec/controllers/projects/labels_controller_spec.rb b/spec/controllers/projects/labels_controller_spec.rb index 622ab154493..7ba4406c1f6 100644 --- a/spec/controllers/projects/labels_controller_spec.rb +++ b/spec/controllers/projects/labels_controller_spec.rb @@ -70,4 +70,19 @@ describe Projects::LabelsController do get :index, namespace_id: project.namespace.to_param, project_id: project.to_param end end + + describe 'POST #generate' do + let(:admin) { create(:admin) } + let(:project) { create(:empty_project) } + + before do + sign_in(admin) + end + + it 'creates labels' do + post :generate, namespace_id: project.namespace.to_param, project_id: project.to_param + + expect(response.code).to eq(302) + end + end end -- cgit v1.2.1 From c0eb2cbbb4b14e3abb843a65d685bee400b5fffe Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Sat, 22 Oct 2016 12:42:19 +0100 Subject: Stop clearing the database cache on rake cache:clear --- lib/tasks/cache.rake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tasks/cache.rake b/lib/tasks/cache.rake index a95a3455a4a..78ae187817a 100644 --- a/lib/tasks/cache.rake +++ b/lib/tasks/cache.rake @@ -29,5 +29,5 @@ namespace :cache do task all: [:db, :redis] end - task clear: 'cache:clear:all' + task clear: 'cache:clear:redis' end -- cgit v1.2.1 From e6968964870286af5ce6a1f7cf1152c057fd5c11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Sat, 22 Oct 2016 13:46:23 +0200 Subject: Fix status code expectation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- spec/controllers/projects/labels_controller_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/controllers/projects/labels_controller_spec.rb b/spec/controllers/projects/labels_controller_spec.rb index 7ba4406c1f6..41df63d445a 100644 --- a/spec/controllers/projects/labels_controller_spec.rb +++ b/spec/controllers/projects/labels_controller_spec.rb @@ -82,7 +82,7 @@ describe Projects::LabelsController do it 'creates labels' do post :generate, namespace_id: project.namespace.to_param, project_id: project.to_param - expect(response.code).to eq(302) + expect(response).to have_http_status(302) end end end -- cgit v1.2.1 From 884d49ea991322f6530b3281a980527cfb909bb7 Mon Sep 17 00:00:00 2001 From: Nur Rony Date: Mon, 24 Oct 2016 13:02:15 +0600 Subject: code formatting corrected --- app/views/projects/issues/show.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 6defd7834bf..bd629b5c519 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -53,7 +53,7 @@ .issue-details.issuable-details - .detail-page-description.content-block{class: ('hide-bottom-border' unless @issue.description.present? )} + .detail-page-description.content-block{ class: ('hide-bottom-border' unless @issue.description.present? ) } %h2.title = markdown_field(@issue, :title) - if @issue.description.present? -- cgit v1.2.1 From 2fc2be9b7b8b80905acccbb74e4788a7e8a03f55 Mon Sep 17 00:00:00 2001 From: Nur Rony Date: Mon, 24 Oct 2016 13:58:46 +0600 Subject: removes extra line for empty milestone description --- app/assets/stylesheets/framework/common.scss | 2 +- app/views/projects/milestones/show.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index 0d00b6a0d9c..800e2dba018 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -373,4 +373,4 @@ table { border-top: 1px solid $border-color; } -.hide-bottom-border { border-bottom: none;} +.hide-bottom-border { border-bottom: none !important; } diff --git a/app/views/projects/milestones/show.html.haml b/app/views/projects/milestones/show.html.haml index c83818e9199..f9ba77e87b5 100644 --- a/app/views/projects/milestones/show.html.haml +++ b/app/views/projects/milestones/show.html.haml @@ -31,7 +31,7 @@ = link_to namespace_project_milestone_path(@project.namespace, @project, @milestone), data: { confirm: 'Are you sure?' }, method: :delete, class: "btn btn-grouped btn-danger" do Delete - .detail-page-description.milestone-detail + .detail-page-description.milestone-detail{ class: ('hide-bottom-border' unless @milestone.description.present? ) } %h2.title = markdown_field(@milestone, :title) %div -- cgit v1.2.1