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

188704452-tweak-ping-times #1063

Merged
merged 2 commits into from
Dec 23, 2024
Merged

188704452-tweak-ping-times #1063

merged 2 commits into from
Dec 23, 2024

Conversation

maciejkocylapc
Copy link
Collaborator

@maciejkocylapc maciejkocylapc commented Dec 20, 2024

What's this PR do?

  • add max limit in API ping times request
  • add validation to collector payload
  • update docs
  • remove unused code

How should this be manually tested?

  • run tests
  • try both collector and ping_times endpoints
  • try to provide incomplete payload to collector and make sure the data does not get saved

What are the relevant tickets?

Task completed checklist:

  • [] Is there appropriate test coverage?
  • [] Did I take into consideration possible security vulnerabilities?
  • [] Are the controllers secure?
  • [] Was every important matter documented?

Things to watch out for

  • [N] Does this PR have any migrations?
  • [N] Does this PR add new dependencies?

@@ -23,6 +24,10 @@ def ping_times_guard
return Pipeline.new(400, p[:payload], 'Wrong payload_version')
end

REQUIRED_PAYLOAD_FIELDS.each do |field|
return Pipeline.new(400, p[:payload], "No payload field: #{field}") unless collector.payload =~ /#{field}/
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

It generally works, but we should check the same for every record in the pipeline (now it's enough that one of them has required fields).

@kbiala kbiala merged commit 41d3a58 into master Dec 23, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants