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

Adjust S3 Plugin for HealthCheck Configuration #61

Merged

Conversation

lukas-kd
Copy link
Contributor

This pull request introduces enhancements to the S3 plugin by adding configuration options for health checks. Specifically:

  • New Feature: Added a HealthCheckPath property to the configuration, allowing more control over the health check process.
  • Health Check Enhancement: Users can now specify a URL health check path to verify a particular endpoint (HealthCheckPath) or fall back to checking only the base domain.

These changes provide increased flexibility in monitoring and ensure that the health check can be tailored to specific URLs or general availability of the domain.

Changes Summary:

  • Added HealthCheckPath property to the S3 plugin configuration.
  • Updated health check logic to support checking the full URL (including HealthCheckPath) or just the base domain, based on configuration.

Testing:

  • Verified configuration updates work as expected.
  • Tested both cases: health checks on the base domain and on a specified HealthCheckPath URL.

Motivation:
This change allows more specific monitoring of S3 instances, improving reliability and reducing false positives when checking the service's health.

Please review and let me know if further adjustments are needed.

Lukas Kürzdörfer and others added 2 commits October 17, 2024 13:38
@lukas-kd
Copy link
Contributor Author

@apurkovic @tst-sia can anyone from you review this pull request?

@tst-sia
Copy link
Contributor

tst-sia commented Oct 22, 2024

Hello @lukas-kd, we reviewed the changes and ran into problems with the Kafka.Flow.Tests project (with regards to Google.Protobuf, protobuf-net packages) aswell as the OpenApiPlugin Tests on our local machines under Windows 10.

We managed to fix some of them and assume we've found part of the culprit for others but we would for now suggest you open a new PR with just the S3 changes you need and no nuget updates (as we would rather separate those right now).

@lukas-kd
Copy link
Contributor Author

@tst-sia i've reverted the package raises.

Do you have any plans for upgrading the nuget packages? Some of them contains vulnerabilities.

Thanks in advance.

@tst-sia
Copy link
Contributor

tst-sia commented Oct 22, 2024

@lukas-kd Thank you, we'll look at upgrading the packages within this or next week. I've approved the S3 changes and merged them.

@tst-sia tst-sia merged commit 6e51c26 into sia-digital:main Oct 22, 2024
@lukas-kd lukas-kd deleted the feature/s3_configuration_healthcheck branch October 22, 2024 14:32
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.

3 participants