influxdata / influxdb-client-csharp

InfluxDB 2.x C# Client
https://influxdata.github.io/influxdb-client-csharp/api/InfluxDB.Client.html
MIT License
360 stars 94 forks source link

feat: add support to filter by `Instant`, `ZonedDateTime`, `OffsetDateTime`, ... in LINQ #605

Closed Sylvain2703 closed 3 months ago

Sylvain2703 commented 10 months ago

Proposed Changes

Features

Refactor

Tests

Checklist

Sylvain2703 commented 10 months ago

Hi @bednar,

Thank you very much for your feedback 😀

Regarding the C# version, I was surprised that this lib still uses C# 8 only... but I understood the reason when I saw the CI errors 😉 Fortunately, we can use newer versions of C# with older versions of .NET as soon as the .NET SDK provides support for the specified C# version. All new C# features that don't require runtime changes will be supported (because the compiler produces compatible IL code with older .NET versions). Unsupported C# features will simply produce compilation errors.

In our case, C# 12 requires the .NET 8.0 SDK but can compile for .NET Core 3.1 (or even .NET Framework 4.*). My main reason for using a newer C# version is the switch expression with logical operators and, or and not (used here) requires C# 9.

After checking the CI pipeline and trying to use the .NET 8 SDK, I realized that the tests cannot run because the .NET SDK Docker image only contains the runtime for the same version. A solution will be to install .NET Core 3.1/5/6/7 runtimes on the .NET 8 SDK Docker image, as mentioned here. What do you think about this approach?

Best regards

bednar commented 9 months ago

Hi @Sylvain2703,

Apologies for my late response.

Thank you for thoroughly checking the CI pipeline and experimenting with the .NET 8 SDK. Your insights into the issue with the .NET SDK Docker image and the inability to run tests due to version-specific runtime constraints are invaluable.

Your proposed solution to install multiple .NET Core runtimes (3.1/5/6/7) on the .NET 8 SDK Docker image, as discussed in this StackOverflow answer, sounds like an excellent approach. It not only resolves our current issue but also simplifies future maintenance by ensuring compatibility across different .NET versions. This foresight is greatly appreciated and aligns well with our goals for the client's long-term usability and robustness.

We are more than happy to incorporate these changes into the client. Your initiative in finding and suggesting a viable solution is a significant contribution to the project. Thank you for your dedication and valuable input.

Best Regards

Sylvain2703 commented 9 months ago

Hi @bednar,

Thank you for your answer!

After experimenting a lot with CircleCI, I propose a slightly different but cleaner solution than the one mentioned previously.

Instead of installing additional .NET runtimes on the .NET 8 SDK Docker image (which was causing dependencies incompatibilities with the base image), I opened PR #613 to:

  1. Build the entire solution, in Release mode, with the .NET 8 SDK,
  2. Copy the resulting build for testing on .NET Core 3.1/5/6/7/8 Docker images and Windows execution environment,
  3. Deploy the tested build (instead of rebuilding/repacking it).

After completing PR #613, this PR should be able to pass the CI pipeline 😀

Best regards

bednar commented 3 months ago

This PR has been closed because it has not had recent activity. Please reopen if this PR is still important to you and you want to continue with them.