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

Support DD EU in reporter #468

Merged
merged 4 commits into from
Nov 28, 2018
Merged

Support DD EU in reporter #468

merged 4 commits into from
Nov 28, 2018

Conversation

remicalixte
Copy link
Contributor

PR to apply on top of #464

@remicalixte remicalixte force-pushed the remicalixte/site-report branch from b8f1b1b to 64a5a41 Compare October 24, 2018 16:42
Copy link
Member

@truthbk truthbk left a comment

Choose a reason for hiding this comment

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

This looks pretty good. We do break the interface for datadog_agent::reports currently, let's make it backward compatible.

@@ -15,6 +15,7 @@
#
class datadog_agent::reports(
$api_key,
$datadog_site,
Copy link
Member

Choose a reason for hiding this comment

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

This would not be backward compatible if referenced on its own. It's fine in the context of init.pp, but if someone has already written a custom manifest using datadog_agent::reports, this would break it. You'll need to add a default (probably an empty string, or the US site).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I see, I didn't know it was a public interface

Copy link
Member

@truthbk truthbk Nov 28, 2018

Choose a reason for hiding this comment

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

This is a weird edge case that might not affect anyone, but it's not hard to fix on our side, so let's better be safe than sorry. 🙇

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be fixed now

@remicalixte remicalixte force-pushed the remicalixte/site-report branch 6 times, most recently from 4ef5507 to 6044e30 Compare November 28, 2018 20:38
@remicalixte remicalixte force-pushed the remicalixte/site-report branch from 6044e30 to fa05e3e Compare November 28, 2018 20:55
Copy link
Member

@truthbk truthbk left a comment

Choose a reason for hiding this comment

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

Awesome! 🍰

@truthbk truthbk merged commit 51934db into master Nov 28, 2018
@truthbk truthbk deleted the remicalixte/site-report branch November 28, 2018 21:14
@truthbk truthbk added this to the 2.4.0 milestone Dec 20, 2018
cegeka-jenkins pushed a commit to cegeka/puppet-datadog_agent that referenced this pull request Apr 6, 2020
* Write api_url in the report config

* Use api_url to choose datadog host

* Update spec tests

* Add a default value for datadog_site
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