Skip to content

[mongodbatlasreceiver] Add baseURL config #39345

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 23 commits into from
May 5, 2025
Merged

[mongodbatlasreceiver] Add baseURL config #39345

merged 23 commits into from
May 5, 2025

Conversation

ArturSantosTD
Copy link
Contributor

Description

Adding a feature - On mongodbatlas receiver, allow the use of cloud.mongodbgov.com instead of only the default cloud.mongodb.com, adding a new config

Documentation

Update mongodbatlas documentation to reflect new config setting

@ArturSantosTD ArturSantosTD requested a review from a team as a code owner April 11, 2025 16:34
@ArturSantosTD ArturSantosTD requested a review from ChrsMark April 11, 2025 16:34
@linux-foundation-easycla Linux Foundation: EasyCLA
Copy link

linux-foundation-easycla bot commented Apr 11, 2025 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@github-actions github-actions bot requested a review from schmikei April 11, 2025 16:35
@atoulme
Copy link
Contributor

atoulme commented Apr 11, 2025

Please add a changelog by typing make chlog-new and following the prompts.

edmocosta
edmocosta reviewed Apr 14, 2025
View reviewed changes
Copy link
Contributor

@edmocosta edmocosta left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution, @ArturSantosTD. I've left a few suggestions for your consideration.

@ArturSantosTD ArturSantosTD requested a review from edmocosta April 14, 2025 16:53
@github-actions github-actions bot mentioned this pull request Apr 15, 2025
Closed
edmocosta
edmocosta reviewed Apr 16, 2025
View reviewed changes
Copy link
Contributor

@edmocosta edmocosta left a comment

Choose a reason for hiding this comment

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

LGTM! @schmikei, your review would be appreciated. Thanks!

schmikei
schmikei suggested changes Apr 16, 2025
View reviewed changes
Copy link
Contributor

@schmikei schmikei left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

My biggest suggestion is making baseURL defined as a default parameter rather than optional.

Please also apply @edmocosta's suggestions on error handling in tests.

@ArturSantosTD ArturSantosTD requested a review from schmikei April 18, 2025 04:15
@github-actions github-actions bot mentioned this pull request Apr 22, 2025
Closed
schmikei
schmikei suggested changes Apr 22, 2025
View reviewed changes
Copy link
Contributor

@schmikei schmikei left a comment

Choose a reason for hiding this comment

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

Overall looks good to me just one more thing with the responsibility of Validate should not modify the underlying struct.

@ArturSantosTD ArturSantosTD requested a review from schmikei April 22, 2025 18:02
@github-actions github-actions bot mentioned this pull request Apr 29, 2025
Closed
ChrsMark
ChrsMark approved these changes Apr 30, 2025
View reviewed changes
Copy link
Member

@ChrsMark ChrsMark left a comment

Choose a reason for hiding this comment

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

LGTM

@ChrsMark
Copy link
Member

@schmikei please take another look when you get the chance so we can proceed with this one.