Implement CLI for backup #83
Conversation
mdedetrich
force-pushed
the
implement-cli-for-backup
branch
3 times, most recently
from
2bba502
to
1df58c4
Compare
mdedetrich
force-pushed
the
implement-cli-for-backup
branch
from
1df58c4
to
a58e5b6
Compare
mdedetrich
changed the base branch from
main
to
restructure-kafka-client-into-backup
mdedetrich
force-pushed
the
implement-cli-for-backup
branch
2 times, most recently
from
3c3bbe4
to
ea0818c
Compare
| } | ||
| } | ||
|
|
||
| val kafkaConsumerSettingsOpt |
There was a problem hiding this comment.
So this is an example of the problems I was talking about earlier where there are multiple ways to configure your app. Specifically for bootstrap servers, this config is either passed in by properties directly into the Apache (not ours or Alpakka's) kafka client which the Alpakka kafka client wraps.
mdedetrich
force-pushed
the
implement-cli-for-backup
branch
from
ea0818c
to
d9c2a14
Compare
|
|
||
| import java.time.temporal.ChronoUnit | ||
|
|
||
| @nowarn("msg=method main in class CommandApp is deprecated") |
There was a problem hiding this comment.
| import java.time.temporal.ChronoUnit | ||
| import java.util.concurrent.atomic.AtomicReference | ||
|
|
||
| class Entry(val initializedApp: AtomicReference[Option[App[_]]] = new AtomicReference[Option[App[_]]](None)) |
There was a problem hiding this comment.
This is super wired, AtomicReference for app, but I think this is the way it should be?
There was a problem hiding this comment.
I tried using a var but I got a Scala compiler warning and I couldn't be bothered figuring it out so I just use an AtomicReference.
Also incase its not clear, the reason why its wired this way is only so we can test the App.
| ) | ||
| name := s"$baseName-cli-backup" | ||
| ) | ||
| .dependsOn(coreCli % "compile->compile;test->test", |
There was a problem hiding this comment.
I am wondering if you need test->test here, do you reuse test classes in there?
There was a problem hiding this comment.
I don't use test-classes in coreCli but I would imagine at some point in the future we would have tests in there for the decline options/arguments (i.e. just the command line parsing part) and there is likely to be some common test functions there.
I will leave it there for now, we can always remove it later if we don't end up doing this.
mdedetrich
force-pushed
the
implement-cli-for-backup
branch
from
f3f4bb6
to
644a97c
Compare
About this change - What it does
This change implements basic CLI interface for the backup client
Why this way
This PR ultimately adds in CLI capability for the guardian backup tool, allowing you to both pass in command line arguments into guardian-backup as well as allowing you to package the tool into various formats. Ideally I would have liked to also create a native image using graalvm but it turns out using graalvm with akka ecosystem doesn't appear to be that well support (mainly due to reflection reasons) but even basic things such as logback causes problems for the same reason so I will leave this for now.
The PR also adds standard basic configuration, i.e. a
logback.xmlso you can see basic logs as well as configuring akka to log through that logback.A basic smoke style test has also been created where we pass cli command line arguments into the app and we check that the configuration setttings correctly passed into the application (note that we are not doing an end2end test because thats the job of other modules).
In terms of publishing formats currently rpm and java
.zipformats are supported. Support for other formats like docker can also be done but to make this PR smaller we can handle this later.Note that not all settings have been configured via CLI. This is due to valuing my sanity because of the sheer amount of things that can be configured via environment variables. Some configurations such as
S3Settingswhich store S3 region/secret/access can also be somewhat reasonably argued should also be configurable by CLI but this improvement can be done for a later PR.