-
Notifications
You must be signed in to change notification settings - Fork 866
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
Binary incompatibility with NJsonSchema version 11.0.0 #2169
Comments
We are experiencing this issue when attempting to update to .NET 8 also |
@NoahStolk Confluent will do nothing. They do not actively support most of their open-source libraries. Just look at elasticsearch and jdbc connectors for instance: they completely ignore issues and PRs. |
That's a shame, and I have already done that and upgraded to .NET 8 successfully with minimal changes. I would however prefer to keep this issue open for anyone else who might run into the same problem. The |
Thanks for reporting. I'll check before the new version release if we can upgrade the NJsonSchema. Please open up a PR if you can, I'll take a look. |
Thanks @anchitj, I'll set up a branch with my current work, and see if I can port the remaining parts of the library to NJsonSchema 11 later this week. 👍 |
I have completed my PR. Build and unit tests are succeeding. Please let me know if there's anything else I can do to get this merged. It would be ideal if the change could be included in a 3.0.0 release. |
Hi @anchitj, are you able to give us an indication of when we can expect a new release? I imagine the PR would not get merged until a new release is scheduled. Is that correct? |
Anything that could be done to expedite this fix/release would be much appreciated by the community, it is a show stopper for .NET8 migration Thanks in advance |
FWIW, .NET 7 has gone out of maintenance earlier this week. Am I correct in assuming that the library authors cannot release breaking changes due to the version number being in sync with librdkafka? What is the versioning policy here? Would this mean that the fix can only get released when librdkafka 3.0.0 comes out, if ever? |
Any update on when this is getting rolled out? We are moving apps from .NET 6 (EOL in Nov) to .NET 8...this fix is critical for us. |
It's been over 4 months since I've opened my PR and I haven't received any replies from Confluent. PR #2226 has recently been merged and now my branch has some merge conflicts. I wouldn't mind fixing those, but since I'm not getting any response I don't think it's worth my time? I understand this is a difficult problem where a major release is required if we are to follow semantic versioning, but not getting any response at all is a little frustrating, especially given the "help appreciated!" label assigned to this issue. Even just saying "we can't fix this because we cannot release breaking changes at this time" would be fine with me. There are other ways around this problem, but the lack of communication does not help. |
@NoahStolk , sorry for the lack of responsiveness. Yes, we want to take this change and are trying to decide how best to do so, even in a minor release. One suggestion that came up is to add preprocessor directives of the form
so that this would only be a breaking change for those using .Net 8.0. Does this work for you? In any case, if you can address the merge conflicts, we can start work toward merging your PR. Sorry again for not getting back to you earlier. |
Hi @rayokota, thank you for your reply. Yes, that would work in our case, but it would still be a breaking change for those targeting .NET 8 and using NJsonSchema v10. For example, an app that does not use NSwag might target .NET 8 and use NJsonSchema v10 for different purposes. Your suggestion gave me a different idea -- would it be acceptable to introduce our own constant for this? Instead of relying on the target framework, we could have a constant named I will be resolving the merge conflicts shortly. |
The original PR broke because the fork source was changed to https://github.com/ah-/confluent-kafka-dotnet/, so I've closed that. Couldn't find a reliable way update the fork source on GitHub. I've opened a new PR: #2263 |
@NoahStolk , after talking with a colleague, I think we will want to use |
Blocking issue for me. Is there a temporary workaround? |
@NathanT02 The easiest way would be to just copy the You only need to modify the project file so it uses NJsonSchema 11 and change a few lines of code in 3 source files. Alternatively, I think you can just clone my fork and copy the source files directly from my branch. Then replace all of your references to the |
Description
Our team is currently running into a problem where the current version of NJsonSchema referenced from the Confluent.SchemaRegistry.Serdes.Json library (10.6.3) is not binary compatible with the resolved version from our binaries (11.0.0). We require NJsonSchema version 11.0.0 due to having a dependency on NSwag.AspNetCore version 14.0.0, and we require NSwag.AspNetCore to be on version 14.0.0 because that is the only version compatible with .NET 8.
NSwag.AspNetCore 14.0.0 is requesting NJsonSchema 11.0.0 from NuGet, which is the highest version and therefore ends up being resolved. However, since Confluent.SchemaRegistry.Serdes.Json is requesting NJsonSchema 10.6.3, we are getting runtime errors such as:
It seems that in NJsonSchema 11.0.0 the
JsonSchemaGeneratorSettings
class in particular has had some major (both source and binary) breaking changes which currently affect the code in the Confluent.SchemaRegistry.Serdes.Json library:ActualSerializerSettings
andSerializerSettings
properties no longer exist..NET 7 will reach end of support on May 14th this year, and besides that we'd very much like to migrate to .NET 8 for various reasons.
What is the recommended approach here? Would it be possible for Confluent.SchemaRegistry.Serdes.Json to migrate to NJsonSchema version 11.0.0? I'd be more than happy to open a PR regarding this issue, but thought it'd be a good idea to discuss here first.
How to reproduce
The program crashes with the following exception:
Checklist
Please provide the following information:
The text was updated successfully, but these errors were encountered: