Skip to content

Conversation

@magnusgundersen
Copy link
Contributor

No description provided.

Copy link
Owner

@tchiotludo tchiotludo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly good.
Please just add :

  • configuration on application.example.yml
  • some doc on README.md about this configuration
  • minor review ⬆️

.registerModule(new JavaTimeModule())
.registerModule(new Jdk8Module())
.setTimeZone(TimeZone.getDefault());
System.out.println("Inclusions: " + jsonInclusions);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops.. done!


public static String toJson(GenericRecord record) throws IOException {
Map<String, Object> map = AvroDeserializer.recordDeserializer(record);
public AvroToJsonSerializer(@Value("${akhq.avro-serializer.json.serialization.inclusions}") @Nullable List<Include> jsonInclusions) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are changing the default configuration, please add old default one on the application.yml please

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the old default to src/main/resources/application.yml

  avro-serializer:
    json.serialization.inclusions:
      - NON_NULL
      - NON_EMPTY

@magnusgundersen magnusgundersen marked this pull request as ready for review August 29, 2021 11:34
@tchiotludo tchiotludo merged commit 2ad056b into tchiotludo:dev Aug 30, 2021
@tchiotludo
Copy link
Owner

Perfect, thanks a lot 👍

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

Successfully merging this pull request may close these issues.

2 participants