search

AbsaOSS / ABRiS

Avro SerDe for Apache Spark structured APIs.
Apache License 2.0
230 stars 75 forks source link

abris #230 improve exception message and comments #231

Closed cerveada closed 3 years ago

cerveada commented 3 years ago

I removed SchemaManagerException. It causes the binary incompatibility errors, but it is not used anywhere, so I would delete it.

At the end I kept the condition as is, seems to me, it is not better reversed more like the same.

kevinwallimann commented 3 years ago

I removed SchemaManagerException. It causes the binary incompatibility errors, but it is not used anywhere, so I would delete it.

At the end I kept the condition as is, seems to me, it is not better reversed more like the same.

I agree with that. The logic is not confusing, only the error message was. "Make sure that the Schema Registry is available, the parameters are correct" This part suggested that schema registry being unavailable could be the cause, which cannot be.

Unfortunately I think you cannot remove the SchemaManagerException due to binary compatibility reasons.

cerveada commented 3 years ago

Unfortunately I think you cannot remove the SchemaManagerException due to binary compatibility reasons.

Are you afraid someone is catching it even though we never throw it? If we want to be extra safe, we can keep it for that case.

kevinwallimann commented 3 years ago

Unfortunately I think you cannot remove the SchemaManagerException due to binary compatibility reasons.

Are you afraid someone is catching it even though we never throw it? If we want to be extra safe, we can keep it for that case.

No, I don't think anybody would do that. Then again, you never know. It's more a question of principles, whether we strictly adhere to binary compatibility and defer all incompatible changes to the next major version or we define a gray area where we accept incompatible changes. Probably we should first implement #159