Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RC:Add apply state and apply error #2973

Merged
merged 3 commits into from
Jul 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion lib/datadog/core/remote/configuration/content.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ def parse(hash)
end
end

attr_reader :path, :data, :hashes
attr_reader :path, :data, :hashes, :apply_state, :apply_error
attr_accessor :version

def initialize(path:, data:)
@path = path
@data = data
@apply_state = ApplyState::UNACKNOWLEDGED
@apply_error = nil
@hashes = {}
@version = 0
end
Expand All @@ -36,6 +38,31 @@ def length
@length ||= @data.size
end

# Sets this configuration as successfully applied.
def applied
@apply_state = ApplyState::ACKNOWLEDGED
marcotc marked this conversation as resolved.
Show resolved Hide resolved
@apply_error = nil
end

# Sets this configuration as not successfully applied, with
# a message describing the error.
def errored(error_message)
@apply_state = ApplyState::ERROR
@apply_error = error_message
end

module ApplyState
# Default state of configurations.
# Set until the component consuming the configuration has acknowledged it was applied.
UNACKNOWLEDGED = 1

# Set when the configuration has been successfully applied.
ACKNOWLEDGED = 2

# Set when the configuration has been unsuccessfully applied.
ERROR = 3
Comment on lines +57 to +63
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: I'm curious, do we need these integers in the protocol? Could they be symbols (:error, :unacknowledged, ...) instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are numbers, as part of the protocol. Backend expects these specific values.

end

private

def compute_and_store_hash(type)
Expand Down
4 changes: 3 additions & 1 deletion lib/datadog/core/remote/configuration/repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@ def contents_to_config_states(contents)
{
id: content.path.config_id,
version: content.version,
product: content.path.product
product: content.path.product,
apply_state: content.apply_state,
apply_error: content.apply_error,
}
end
end
Expand Down
14 changes: 14 additions & 0 deletions sig/datadog/core/remote/configuration/content.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,28 @@ module Datadog

attr_accessor version: Integer

attr_reader apply_state: Integer

attr_reader apply_error: String?

@length: Integer

def initialize: (path: Configuration::Path, data: StringIO) -> void

def applied: -> void

def errored: (String error_message) -> void

def hexdigest: (Symbol type) -> String

def length: () -> Integer

module ApplyState
ACKNOWLEDGED: Integer
ERROR: Integer
UNACKNOWLEDGED: Integer
end

private

def compute_and_store_hash: (Symbol type) -> String
Expand Down
28 changes: 28 additions & 0 deletions spec/datadog/core/remote/configuration/content_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -264,5 +264,33 @@
end
end
end

describe '#applied' do
subject(:applied) { content.applied }

it 'sets applied_state to acknowledged' do
applied
expect(content.apply_state).to eq(2)
end

it 'clear errors' do
content.errored('error message')

applied

expect(content.apply_error).to be_nil
end
end

describe '#errored' do
subject(:errored) { content.errored(message) }
let(:message) { 'test-message' }

it 'sets applied_state to error with message' do
errored
expect(content.apply_state).to eq(3)
expect(content.apply_error).to eq('test-message')
end
end
end
end
4 changes: 2 additions & 2 deletions spec/datadog/core/remote/configuration/respository_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@
context 'with changes' do
let(:expected_config_states) do
[
{ :id => path.config_id, :product => path.product, :version => 1 }
{ :id => path.config_id, :product => path.product, :version => 1, apply_error: nil, apply_state: 1 }
]
end

Expand All @@ -397,7 +397,7 @@
end

expected_updated_config_states = [
{ :id => path.config_id, :product => path.product, :version => 2 }
{ :id => path.config_id, :product => path.product, :version => 2, apply_error: nil, apply_state: 1 }
]

expect(repository.state.config_states).to_not eq(expected_config_states)
Expand Down