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

Change directory parameter to optional #10051

Merged
merged 8 commits into from
Sep 13, 2021

Conversation

steveny91
Copy link
Contributor

What does this PR do?

This PR makes the directory parameter in the postfix check optional to keep backwards compatibility.

Motivation

In check version < 1.9, there wasn't any validation for the check, which allowed the check to work without setting a directory parameter. In the newest version (1.9) there's a validator that checks that directory is set and this breaks existing configuration in which postqueue: true is set and directory is omitted.

Additional Notes

Also added some explaining text in the conf.yaml.example and updated the readme accordingly.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached

@github-actions
Copy link

github-actions bot commented Sep 3, 2021

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

@steveny91 steveny91 changed the title [Postfix] Make the directory parameter optional Made the directory parameter optional Sep 3, 2021
@steveny91 steveny91 changed the title Made the directory parameter optional Change directory parameter optional Sep 3, 2021
@github-actions
Copy link

github-actions bot commented Sep 3, 2021

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

@steveny91 steveny91 changed the title Change directory parameter optional Change directory parameter to optional Sep 3, 2021
@codecov
Copy link

codecov bot commented Sep 3, 2021

Codecov Report

Merging #10051 (cb9ad3f) into master (b7f9e13) will increase coverage by 0.04%.
The diff coverage is n/a.

Flag Coverage Δ
postfix 88.04% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

postfix/assets/configuration/spec.yaml Outdated Show resolved Hide resolved
postfix/assets/configuration/spec.yaml Outdated Show resolved Hide resolved
Changed for consistency

Co-authored-by: Fanny Jiang <fanny.jiang@datadoghq.com>
@github-actions
Copy link

github-actions bot commented Sep 3, 2021

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

fanny-jiang
fanny-jiang previously approved these changes Sep 3, 2021
@fanny-jiang fanny-jiang mentioned this pull request Sep 3, 2021
4 tasks
alai97
alai97 previously approved these changes Sep 7, 2021
Copy link
Contributor

@alai97 alai97 left a comment

Choose a reason for hiding this comment

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

Looks good pending the following suggestions (removing "please")!

postfix/assets/configuration/spec.yaml Outdated Show resolved Hide resolved
postfix/datadog_checks/postfix/data/conf.yaml.example Outdated Show resolved Hide resolved
@steveny91 steveny91 dismissed stale reviews from alai97 and fanny-jiang via a6ef715 September 7, 2021 20:33
@ChristineTChen ChristineTChen merged commit 40c7cd6 into master Sep 13, 2021
@ChristineTChen ChristineTChen deleted the steveny91/postfix_directory_fix branch September 13, 2021 21:51
@coignetp coignetp mentioned this pull request Oct 5, 2021
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants