FINERACT-2149: disable liquibase phone home #4177
Conversation
meonkeys
changed the title
|
Squash your commits |
I'm new and naïve... please scrutinize carefully and expect beginner mistakes. See related discussion at https://lists.apache.org/thread/5s7zjsxdcqk1qvjnc1wbhqqv66shpl54 --- set liquibase.analytics.enabled in top-level main() This seems like the best place to set this globally. Surely I could be doing something wrong or silly: Please advise. I'm rusty and unfamiliar with class/bean instantiation order and process. My naïve test/check is visually inspecting the output of the log message I added in TenantDatabaseUpgradeService to see if `liquibase.analytics.enabled` is set, and to what. I'm happy to automate this, but it seemed unnecessary? Advice welcome there, too. I only see `false` for `liquibase.analytics.enabled` value when I set it in ServerApplication. I tried setting `liquibase.analytics.enabled` to `false` in ExtendedSpringLiquibase and ExtendedSpringLiquibaseFactory constructors but that didn't work... perhaps these are not instantiated that way, or in time? Arnold did mention we use a custom trigger, not Sping Boot ( https://lists.apache.org/thread/rqrohj0q2ql3k89b52gq1rnj1x81ktcm ), and maybe these classes are only relevant to Spring Boot. I can't use a simple public constructor in TenantDatabaseUpgradeService, that causes something to throw a fatal runtime exception: `error: variable tenantDetailsService might not have been initialized`. I could set the property in one of several other methods, but which one? For posterity, we could also use an environment variable, I'm also just not sure where that should happen if we did. --- don't log default value for liquibase opt-out This allows us to see both * whether the value is set or not (will log "null" if it isn't) * what the value is set to, if it is set
meonkeys
force-pushed
the
disable-liquibase-phone-home
branch
from
19006d7
to
6a486a3
Compare
| @@ -54,6 +54,7 @@ private static SpringApplicationBuilder configureApplication(SpringApplicationBu | |||
| } | |||
|
|
|||
| public static void main(String[] args) throws IOException { | |||
| System.setProperty("liquibase.analytics.enabled", "false"); | |||
There was a problem hiding this comment.
wouldn't it be placed more strategically at the TenantDatabaseUpgradeService itself, in a static initializer? Can you test it if it works the same way there?
There was a problem hiding this comment.
There was a problem hiding this comment.
@kjozsa: yes, it does.
@kjozsa and @IOhacker: how about we go with just this simple static initializer in TenantDatabaseUpgradeService:
static {
System.setProperty("liquibase.analytics.enabled", "false");
}I can't imagine anyone wanting to set the flag to true, and it would be many fewer lines of code just to hardcode it to "false" in this manner rather than provide a configurable setting. If there's demand to make it configurable later, we could of course do that. I feel like our use of Liquibase is up to us, though. An under the hood / upstream decision we get to make. Thoughts?
Also, is it clear that we must use System.setProperty? Our only other alternative is to use an environment setting LIQUIBASE_ANALYTICS_ENABLED=false, visible to the Java VM before/as it initializes. I don't see any way in the Liquibase API to disable phoning home at runtime.
Description
This patch globally disables Liquibase's phone home / telemetry using their recommended method.
I'm new and naïve... please see my commits and commit logs, scrutinize carefully, and expect beginner mistakes.
See related discussion at https://lists.apache.org/thread/5s7zjsxdcqk1qvjnc1wbhqqv66shpl54
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!