Commit e63d5187 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Use feature_category :not_owned for workers

This removes the feature_category_not_owned! method from workers, and
used the `feature_category :not_owned` approach instead.

The latter approach is more in line with what we've done for Grape
endpoints and controller endpoints.

This also means that `feature_category :not_owned` will be caught by
our RuboCop.
parent d2b42c0a
......@@ -5,6 +5,6 @@ module ChaosQueue
included do
queue_namespace :chaos
feature_category_not_owned!
feature_category :not_owned # rubocop:todo Gitlab/AvoidFeatureCategoryNotOwned
end
end
......@@ -8,7 +8,10 @@ module ReactiveCacheableWorker
sidekiq_options retry: 3
feature_category_not_owned!
# Feature category is different depending on the model that is using the
# reactive cache. Identified by the `related_class` attribute.
feature_category :not_owned # rubocop:todo Gitlab/AvoidFeatureCategoryNotOwned
loggable_arguments 0
def self.context_for_arguments(arguments)
......
......@@ -35,17 +35,9 @@ module WorkerAttributes
class_methods do
def feature_category(value, *extras)
raise "Invalid category. Use `feature_category_not_owned!` to mark a worker as not owned" if value == :not_owned
set_class_attribute(:feature_category, value)
end
# Special case: mark this work as not associated with a feature category
# this should be used for cross-cutting concerns, such as mailer workers.
def feature_category_not_owned!
set_class_attribute(:feature_category, :not_owned)
end
# Special case: if a worker is not owned, get the feature category
# (if present) from the calling context.
def get_feature_category
......
......@@ -7,7 +7,7 @@ class DeleteStoredFilesWorker # rubocop:disable Scalability/IdempotentWorker
sidekiq_options retry: 3
feature_category_not_owned!
feature_category :not_owned # rubocop:todo Gitlab/AvoidFeatureCategoryNotOwned
loggable_arguments 0
def perform(class_name, keys)
......
......@@ -12,7 +12,10 @@ class FlushCounterIncrementsWorker
sidekiq_options retry: 3
feature_category_not_owned!
# The increments in `ProjectStatistics` are owned by several teams depending
# on the counter
feature_category :not_owned # rubocop:disable Gitlab/AvoidFeatureCategoryNotOwned
urgency :low
deduplicate :until_executing, including_scheduled: true
......
......@@ -8,7 +8,7 @@ module ObjectStorage
include ObjectStorageQueue
sidekiq_options retry: 5
feature_category_not_owned!
feature_category :not_owned # rubocop:todo Gitlab/AvoidFeatureCategoryNotOwned
loggable_arguments 0, 1, 2, 3
def perform(uploader_class_name, subject_class_name, file_field, subject_id)
......
......@@ -10,7 +10,7 @@ module ObjectStorage
sidekiq_options retry: 3
include ObjectStorageQueue
feature_category_not_owned!
feature_category :not_owned # rubocop:todo Gitlab/AvoidFeatureCategoryNotOwned
loggable_arguments 0, 1, 2, 3
SanityCheckError = Class.new(StandardError)
......
......@@ -58,7 +58,8 @@ not, the specs will fail.
### Excluding Sidekiq workers from feature categorization
A few Sidekiq workers, that are used across all features, cannot be mapped to a
single category. These should be declared as such using the `feature_category_not_owned!`
single category. These should be declared as such using the
`feature_category :not_owned`
declaration, as shown below:
```ruby
......@@ -66,7 +67,7 @@ class SomeCrossCuttingConcernWorker
include ApplicationWorker
# Declares that this worker does not map to a feature category
feature_category_not_owned!
feature_category :not_owned # rubocop:disable Gitlab/AvoidFeatureCategoryNotOwned
# ...
end
......
......@@ -12,7 +12,7 @@ module RuboCop
RESTRICT_ON_SEND = %i[feature_category get post put patch delete].freeze
def_node_matcher :feature_category_not_owned?, <<~PATTERN
(send nil? :feature_category (sym :not_owned) ...)
(send _ :feature_category (sym :not_owned) ...)
PATTERN
def_node_matcher :feature_category_not_owned_api?, <<~PATTERN
......
......@@ -292,7 +292,7 @@ RSpec.describe Gitlab::SidekiqMiddleware::ServerMetrics do
if category
feature_category category
else
feature_category_not_owned!
feature_category :not_owned
end
def perform
......
......@@ -28,7 +28,7 @@ RSpec.describe Gitlab::SidekiqMiddleware::WorkerContext::Client do
'TestNotOwnedWithContextWorker'
end
feature_category_not_owned!
feature_category :not_owned
end
end
......
......@@ -29,7 +29,7 @@ RSpec.describe Gitlab::SidekiqMiddleware::WorkerContext::Server do
"NotOwnedWorker"
end
feature_category_not_owned!
feature_category :not_owned
end
end
......
......@@ -20,6 +20,13 @@ RSpec.describe RuboCop::Cop::Gitlab::AvoidFeatureCategoryNotOwned do
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid adding new endpoints with `feature_category :not_owned`. See https://docs.gitlab.com/ee/development/feature_categorization
SOURCE
end
it 'flags a method call on a class with an array passed' do
expect_offense(<<~SOURCE)
worker.feature_category :not_owned, [:index, :edit]
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid adding new endpoints with `feature_category :not_owned`. See https://docs.gitlab.com/ee/development/feature_categorization
SOURCE
end
end
context 'in controllers' do
......
......@@ -49,7 +49,7 @@ RSpec.describe ApplicationWorker do
worker.feature_category :pages
expect(worker.sidekiq_options['queue']).to eq('queue_2')
worker.feature_category_not_owned!
worker.feature_category :not_owned
expect(worker.sidekiq_options['queue']).to eq('queue_3')
worker.urgency :high
......
......@@ -54,7 +54,7 @@ RSpec.describe 'Every Sidekiq worker' do
# All Sidekiq worker classes should declare a valid `feature_category`
# or explicitly be excluded with the `feature_category_not_owned!` annotation.
# Please see doc/development/sidekiq_style_guide.md#feature-categorization for more details.
it 'has a feature_category or feature_category_not_owned! attribute', :aggregate_failures do
it 'has a feature_category attribute', :aggregate_failures do
workers_without_defaults.each do |worker|
expect(worker.get_feature_category).to be_a(Symbol), "expected #{worker.inspect} to declare a feature_category or feature_category_not_owned!"
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment