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

feat: Implement Monitoring Service #729

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

adeatcu-ionos
Copy link
Contributor

@adeatcu-ionos adeatcu-ionos commented Dec 18, 2024

What does this fix or implement?

Checklist

  • PR name added as appropriate (e.g. feat:/fix:/doc:/test:/refactor:)
  • Tests added or updated
  • Documentation updated
  • Changelog updated and version incremented (label: upcoming release)
  • Github Issue linked if any
  • Jira task updated

@adeatcu-ionos
Copy link
Contributor Author

adeatcu-ionos commented Dec 18, 2024

Some notes about this PR:

  • The structure may not be the best, in terms of where to store the service, the resources/data-sources, etc but I followed the bucket implementation because I just wanted to integrate the service and make it work. I tried to change the structure but I realized that it's a lot of work and it was out of scope for this PR. I will leave this as it is and we will get back to it later.
  • The sentence above also applies for the tests. Ideally, for tests, we would need to make a provider instance so we can reference all the clients that we usually instantiate inside a provider.
  • The tests are not done yet, I still need to add some more
  • The SDK is generated locally, I will make a bundle release soon and then I will modify the PR accordingly.

L.E:

  • All tests were added
  • The local SDK is no longer used, I made a release in bundle and I'm using that release

@adeatcu-ionos adeatcu-ionos force-pushed the implement-monitoring-service branch 2 times, most recently from 7cb3172 to 52b7617 Compare December 18, 2024 15:04
@adeatcu-ionos adeatcu-ionos force-pushed the implement-monitoring-service branch 2 times, most recently from 4a50080 to 4a98774 Compare December 18, 2024 16:17
@adeatcu-ionos adeatcu-ionos force-pushed the implement-monitoring-service branch from 4a98774 to ca005ff Compare December 18, 2024 16:20
…to implement-monitoring-service

# Conflicts:
#	internal/framework/services/monitoring/resource_monitoring_pipeline.go
#	services/monitoring/pipeline.go
@@ -84,12 +84,6 @@ func PreCheck(t *testing.T) {
t.Fatalf("%s/%s or %s must be set for acceptance tests", envar.IonosUsername, envar.IonosPassword, envar.IonosToken)
}
}

accessKey := os.Getenv(envar.IonosS3AccessKey)
Copy link
Contributor Author

@adeatcu-ionos adeatcu-ionos Jan 8, 2025

Choose a reason for hiding this comment

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

@cristiGuranIonos It's good that you removed this since the API will complain about not setting the accessKey and the secretKey, so there is no need for us to do this check.

Copy link

sonarqubecloud bot commented Jan 8, 2025

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