Skip to content
This repository was archived by the owner on Feb 23, 2023. It is now read-only.

Add more support with Spring for Apache Kafka #921

Closed
wants to merge 3 commits into from

Conversation

artembilan
Copy link
Contributor

  • Add missed JsonDeserializer into type hints
  • Add @InitializationHint for some spring-kafka JSON components
    which need to initialize their static props for ClassUtils.isPresent()
  • Improve kafka sample for JSON data interaction
  • Add a @TypeHint for end-user data classes used in the sample
  • Improve docker-compose.yml for extra image of the current kafka sample

@artembilan
Copy link
Contributor Author

Depends on: spring-projects/spring-kafka#1881

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 22, 2021
@artembilan
Copy link
Contributor Author

/CC @garyrussell

return args -> {
template.send("graal", "foo");
template.send("graal", new Greeting("Hello from GraalVM!"));
System.out.println("++++++Sent:foo");
Copy link
Contributor

Choose a reason for hiding this comment

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

Change this to match?

@garyrussell
Copy link
Contributor

Didn't you test it?

You need to update the verify.sh https://github.com/spring-projects-experimental/spring-native/blob/main/samples/kafka/verify.sh

@artembilan
Copy link
Contributor Author

Huh? What is that? Never used those .sh. Tried build.sh once, but it was silent and in the end it produces an .exe which is not always working on Windows.
Yeah.. Thanks for the pointer! Let me see what that verify.sh does for us and fix it respectively!
Docs don't mention this file though...

* Add missed `JsonDeserializer` into type hints
* Add `@InitializationHint` for some `spring-kafka` JSON components
which need to initialize their `static` props for `ClassUtils.isPresent()`
* Improve `kafka` sample for JSON data interaction
* Add a `@TypeHint` for end-user data classes used in the sample
* Improve `docker-compose.yml` for extra image of the current `kafka` sample
* Fix `verify.sh` for expectations after image run
@artembilan
Copy link
Contributor Author

Repushed after rebasing to main.
Checked locally with those build.sh and verify.sh: works well even on Windows for me ! 😄

@garyrussell
Copy link
Contributor

Hmmm - on MacOS:

***************************
APPLICATION FAILED TO START
***************************

Description:

Native reflection configuration for org.springframework.kafka.support.serializer.JsonDeserializer is missing.

@artembilan
Copy link
Contributor Author

Correct. You must build and install locally the main project first.
That JsonDeserializer was missed in the original KafkaHints, so I added it as a part of my PR.

@garyrussell
Copy link
Contributor

Ah; ok; sorry.

Copy link
Contributor

@garyrussell garyrussell left a comment

Choose a reason for hiding this comment

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

LGTM; however since it needs a spring-kafka snapshot, it can't be merged unless there are no plans for a release before August 16 (when spring-kafka 2.7.5 will be released). But I could consider releasing it earlier than that, if needed.

@artembilan
Copy link
Contributor Author

Well, there is no strong dependency on spring-kafka in the main project. Right now it is only a matter of a sample. spring-native can be released as is.
We may find something else though for better compatibility before that August 16th...

@sdeleuze sdeleuze added this to the 0.11.0 milestone Jul 23, 2021
@sdeleuze sdeleuze added type: compatibility Native image compatibility issue and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jul 23, 2021
@sdeleuze sdeleuze modified the milestones: 0.11.0-M1, 0.11.x, 0.10.4 Sep 24, 2021
sdeleuze pushed a commit to sdeleuze/spring-native that referenced this pull request Sep 27, 2021
@sdeleuze sdeleuze closed this in a9ddd12 Sep 27, 2021
sdeleuze pushed a commit that referenced this pull request Sep 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: compatibility Native image compatibility issue
Development

Successfully merging this pull request may close these issues.

4 participants