kubernetes-client icon indicating copy to clipboard operation
kubernetes-client copied to clipboard

fix #4109: moving template handling to a custom deserializer

Open shawkins opened this issue 3 years ago • 3 comments

Description

This addresses #4109 and is related to #3972. It is also a replacement for #4034 - since the idea of subclassing ObjectMapper is unpalatable, the other way to do this is with a custom deserializer. But to keep the same unmatched logic, we need to move that to model-common, rather than client-api.

If it's acceptable, then I can further add a changelog about moving the unmatched static methods, etc. The unmatched test references thing from the api, so it can't be moved to model-common as is - but it could of course be further refactored to move the tests.

With this change we also don't need to have the parameter handling logic so deeply coupled with the Serialization logic - that is we don't have to do string substitution on the string prior to parsing. That could easily be done as post processing logic in the TemplateOperationsImpl.

Type of change

  • [x] Bug fix (non-breaking change which fixes an issue)
  • [ ] Feature (non-breaking change which adds functionality)
  • [x] Breaking change (fix or feature that would cause existing functionality to change
  • [ ] Chore (non-breaking change which doesn't affect codebase; test, version modification, documentation, etc.)

Checklist

  • [x] Code contributed by me aligns with current project license: Apache 2.0
  • [ ] I Added CHANGELOG entry regarding this change
  • [x] I have implemented unit tests to cover my changes
  • [ ] I have added/updated the javadocs and other documentation accordingly
  • [ ] No new bugs, code smells, etc. in SonarCloud report
  • [ ] I tested my code in Kubernetes
  • [ ] I tested my code in OpenShift

shawkins avatar Jul 22 '22 10:07 shawkins

I need to check, but I think this one would break the feature where we allow deserializing fields with unmatched types to the additionalProperties map.

manusa avatar Jul 25 '22 07:07 manusa

I need to check, but I think this one would break the feature where we allow deserializing fields with unmatched types to the additionalProperties map.

Other than changing the package, it does not. You can of course leave the old ones in place, but deprecated if you want.

shawkins avatar Jul 25 '22 11:07 shawkins

I need to check, but I think this one would break the feature where we allow deserializing fields with unmatched types to the additionalProperties map.

Other than changing the package, it does not. You can of course leave the old ones in place, but deprecated if you want.

I just gave it a quick look and the changes in the Serialization class made me think of this. I'll try to review properly later

manusa avatar Jul 25 '22 13:07 manusa

@sunix I mentioned something about backporting on the call - I was mixed up. #4263 is the one that should be considered for backport, not this one.

shawkins avatar Aug 16 '22 18:08 shawkins