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

Fix SerializerConfigBuilder class createGroup method setting invalid FullTransitive compatibility for non-Avro formats by default. #142

Open
jingerbread opened this issue Oct 6, 2020 · 0 comments
Assignees

Comments

@jingerbread
Copy link
Contributor

jingerbread commented Oct 6, 2020

Problem description
Fix SerializerConfigBuilder class createGroup method setting invalid FullTransitive compatibility for non-Avro formats by default.
(packet io.pravega.schemaregistry.serializer.shared.impl )

F.e this group creation will fail with "BadArgumentException: Group properties invalid" for non-Avro formats

SerializerConfig serializerConfig = SerializerConfig.builder()
                .groupId(schemaRegistryConfig.groupId).registryConfig(config)
                .createGroup(SerializationFormat.Json).registerSchema(true)
                .build();

Because by default createGroup method sets invalid FullTransitive compatibility.

Problem location

In this case from "BadArgumentException: Group properties invalid" Exception it is not obvious that problem is with compatibility.
In logs there will be only following

 [RestServer RUNNING] WARN  i.p.s.s.r.r.AbstractResource - Bad argument for request createGroup failed with exception: . null. {}

May be we should add some log.error message before return to isValidCompatibilityForFormat method for Protobuf/Custom/Json/Any cases:

 case Avro:
                return true;
            case Protobuf:
                // Only Allow Any or Deny All are allowed values. 
            case Json:
                // Only Allow Any or Deny All are allowed values. 
            case Custom:
                // Only Allow Any or Deny All are allowed values. 
            case Any:
                boolean isCompatibilityValid = compatibility.getType().equals(Compatibility.Type.AllowAny) || compatibility.getType().equals(Compatibility.Type.DenyAll);
                if (! isCompatibilityValid) {
                       log.error("Invalid compatibility type for  serializationFormat {}, only Allow Any or Deny All is possible for this format");
                }
               return  isCompatibilityValid;
...

Suggestions for an improvement

@jingerbread jingerbread changed the title Fix create default group failed with "BadArgumentException: Group properties invalid" Excepiton for non-Avro formats, Fix "BadArgumentException: Group properties invalid" for non-Avro formats when creating group without specifying Compatibility type Oct 6, 2020
@jingerbread jingerbread changed the title Fix "BadArgumentException: Group properties invalid" for non-Avro formats when creating group without specifying Compatibility type Fix SerializerConfigBuilder class createGroup method setting invalid FullTransitive compatibility for non-Avro formats by default. Oct 6, 2020
@shiveshr shiveshr self-assigned this Oct 6, 2020
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

No branches or pull requests

2 participants