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

Add extendable controller & configuration for setup #28

Merged
merged 2 commits into from
Feb 25, 2024

Conversation

KiKoS0
Copy link
Collaborator

@KiKoS0 KiKoS0 commented Feb 25, 2024

I think someone with better spring boot knowledge can shape
a better interface to create the Inngest client + CommHandler but
this is already better than manually creating a singleton and
having everything in a random controller.

Note: It might be better to not have a second adapter package and
instead have these spring boot helpers as a feature variant. But
I'll see how it goes with this PR first: #27

I'm also adding a way to build the Inngest client configuration
possibly through a Builder pattern targeted to the specific
environment that the user wants to use.

The current interface (which needs better code documentation) is that a consumer application would subclass the controller and configuration as follows:

ExampleController.java
@RestController
@RequestMapping(value = "/api/inngest")
public class ExampleController extends InngestController {
}
ExampleConfiguration.java
@Configuration
public class ExampleConfiguration extends InngestConfiguration {

    @Override
    public HashMap<String, InngestFunction> functions() {
        HashMap<String, InngestFunction> functions = new HashMap<>();
        ...
        return functions;
    }

    @Override
    public Inngest inngestClient() {
        return new Inngest("spring_example");
    }
}

@KiKoS0 KiKoS0 self-assigned this Feb 25, 2024
@KiKoS0 KiKoS0 force-pushed the extendable-controller branch 2 times, most recently from 8c83b9d to 7d37fac Compare February 25, 2024 09:19
@KiKoS0 KiKoS0 changed the title Add extendable controller & configuration to configure inngest Add extendable controller & configuration for setup Feb 25, 2024
@KiKoS0 KiKoS0 marked this pull request as ready for review February 25, 2024 09:36
@KiKoS0 KiKoS0 force-pushed the extendable-controller branch 2 times, most recently from fec731e to 8e0df9d Compare February 25, 2024 09:39
Copy link
Contributor

@darwin67 darwin67 left a comment

Choose a reason for hiding this comment

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

This is fine for now.
The idea of #27 is that there will be a serve function that will handle all the routes, and also take the Inngest client as an input so the route handler will have all the information it needs.

Having some compilation error locally, so haven't push the latest code up yet.

@darwin67
Copy link
Contributor

this is the latest code, and kotlin is not happy about something, but can't tell what it is.
https://github.com/inngest/inngest-kt/pull/27/files#diff-a53270cbe78c3842c55b0a20286bd726f41e9e697070455fb5f75c0fb73a4104R15

@KiKoS0
Copy link
Collaborator Author

KiKoS0 commented Feb 25, 2024

oh it was the Kotlin paratrooped it. 😏

I think someone with better spring boot knowledge can shape
a better interface to create the inngest client + CommHandler but
this is already better than manually creating a singleton and
having everything in a random controller.

Note: It might be better to not have a second adapter package and
instead have these spring boot helpers as a feature variant. But
I'll see how it goes with this PR first:

#27

I'm also adding a way to build the Inngest client configuration
possibly through a Builder pattern targeted to the specific
environment that the user wants to use.
@darwin67
Copy link
Contributor

yeah, I just figured it out. annoying

@KiKoS0 KiKoS0 merged commit 1ac65fc into main Feb 25, 2024
7 checks passed
@KiKoS0 KiKoS0 deleted the extendable-controller branch February 25, 2024 10:00
}

group = "com.inngest"
version = "0.0.1-SNAPSHOT"
Copy link
Collaborator

Choose a reason for hiding this comment

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

probably best to version all the subpackages of inngest-kt together. I think there's something we can add in the root gradle file. I'll take a note of this for now


import com.inngest.*;
import com.inngest.springboot.InngestConfiguration;
import kotlin.jvm.functions.Function2;
Copy link
Collaborator

Choose a reason for hiding this comment

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

we already discussed this but probably ideal if we don't have to pull in a kotlin Function here and can instead use lambdas (available in Java 8) or something like https://docs.oracle.com/javase/8/docs/api/java/lang/Runnable.html

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.

3 participants