-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat: Standalone deployment #328
base: main
Are you sure you want to change the base?
Conversation
|
||
<parent> | ||
<groupId>io.akka</groupId> | ||
<artifactId>akka-javasdk-maven</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we can move all common things to the grand-parent, but I wanted to make sure we agree on the approach of using a separate parent pom first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering if we could do it with a profile instead? Same parent pom, just a different config for the docker-maven-plugin
.. then no changes would be needed to the app. One could decide to package for akka platform or for standalone via profile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea. I have changed to a profile.
2dd9514
to
25fd3aa
Compare
<groupId>io.fabric8</groupId> | ||
<artifactId>docker-maven-plugin</artifactId> | ||
<version>${docker-maven-plugin.version}</version> | ||
<configuration combine.self="override"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
combine.self="override"
is important, otherwise it will merge with the other plugin definition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
filename is misspelled
If you use Kafka as message broker the configuration can be defined in your `application.conf`: | ||
|
||
``` | ||
kalix.proxy.eventing { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunate that the old config structure becomes public API now, can we avoid that by prioritizing some alias trixery or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, the internal config structure that we expect users to touch now becomes public API that will require the same kind of compatibility care as the Akka libraries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, I was about to ask about that. We could probably expose it as:
akka.runtime.broker {
support = kafka
kafka {
}
}
I think broker
is more what it is about, than eventing
, any other opinions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be better if we had a real sample with k8 config rather than the doc-snippets which is just a random set of components to show of small sections of features. For when users are trying this out and want to see something deployed and running?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure it's worth an additional sample, comparing to just adding those files to an existing sample (shopping-cart-quickstart) when trying it out.
* build image via standalone profile * example of kubernetes deployment
(cherry picked from commit 3e58e3f)
37cb3de
to
e39041f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments below, but maybe some of those we can address in next iteration.
LGTM anyway.
@@ -1,9 +1,27 @@ | |||
= Operating | |||
= Fully-automated regions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this could be Fully-managed Operations
... vs Self-managed Operations
(below).. makes the contrast easier to understand in my view.
Co-authored-by: Eduardo Pinto <[email protected]>
No description provided.