Skip to content
This repository has been archived by the owner on Feb 8, 2019. It is now read-only.

Conversation

fanf
Copy link
Member

@fanf fanf commented Sep 3, 2017

https://www.rudder-project.org/redmine/issues/11035

So, this is the basic check for plugin enable-ity. It is horrible. The part for limitation is actually pretty straighfoward, we just forbid data sources fetch to be triggered (with a log, because we are not animals!)
But the build & module init, gods... Maven fault, mainly.
So logic is that we want to let the standard OSS build not be impacted by that, ie: "git clone; mvn compile" should just compile.
For that, we need to let the build be able to choose what checker impl to use, and then we need to uncouple the check loading/init from what impl was chosen. For the first, that means maven profiles and adding sources to compile, and it's horrible.
For the second, it's basically what IoC provides, but no way I will use spring anymore (and it won't be serious to have a second IoC framework in one little plugin), so I went with the convention that the build is doing the right thing(tm), and we actually have the same package/class name for both implementation and only one is compiled into the jar.
That also make it easier to use the correct impl for sure for a given jar: there is no choice past build.

Not to merge until depencies project are settled, but give an overview of what is done.

@fanf fanf force-pushed the ust_11035/add_the_possibility_to_enable_disable_the_plugin_globally branch from 2e39ac4 to 228fb04 Compare September 3, 2017 22:47
@fanf
Copy link
Member Author

fanf commented Sep 3, 2017

PR rebased

import bootstrap.liftweb.{ RudderConfig => Cfg }

// by build convention, we have only one of that on the classpath
lazy val pluginStatusService = new CheckRudderPluginDatasourcesEnableImpl()
Copy link
Member

Choose a reason for hiding this comment

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

Where should be the Impl ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added back.

scheduledTask = Some(source.subscribe())
} else {
// the plugin is disabled, does nothing
DataSourceLogger.warn(s"The datasource with id '${datasource.id.value}' is enabled but the plugin is disabled (reason: ${pluginStatus.enabledStatus}. Not scheduling future runs for it.")
Copy link
Member

Choose a reason for hiding this comment

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

so we'll need to restart the webapp in this case if we correct the enabling file ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, not because of that because you could just re-enable the plugin in the UI or query update from API, and it would restart the scheduler.
But the license file is only read at boot time. That seems an acceptable tradeoff for performance and complexity for a minimal v1.

Actually, I could time time spent in the license evaluation to see if it matches the timeframe of datasources and do it each time. That would also mean that each plugin may ends with a different strategy.

Actually, the correct situation is to have Rudder provides a proxy for licenses with cache, and an update command which read again license. On v2 :)

@fanf fanf force-pushed the ust_11035/add_the_possibility_to_enable_disable_the_plugin_globally branch from 228fb04 to f95af49 Compare September 11, 2017 15:28

def isEnabled(): Boolean

def enabledStatus(): String //not a string actually
Copy link
Member

Choose a reason for hiding this comment

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

In a v2 we could replace by a PluginState trait, which could have as many subclass as we need

pom.xml Outdated
<profile>
<id>internal-limited</id>
<activation>
<!-- Activation via *presece* of a system property to ensure mutual exclusivity
Copy link
Member

Choose a reason for hiding this comment

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

presence (nitpicker ...)

final class CheckRudderPluginDatasourcesEnableImpl() extends CheckRudderPluginDatasourcesEnable {
// here are processed variables
val CLASSPATH_KEYFILE = "license.pubkey" // "${plugin-resource-publickey}"
val FS_SIGNED_LICENSE = "/home/fanf/Rudder/license/license.sign" // "${plugin-resource-license}"
Copy link
Member

Choose a reason for hiding this comment

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

How will this variables be filled with correct value ?? I Think they will be injected at build time

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, see the makefile. We are filtering the source file (that's why it is in template, then go to target/generated, then compiled) and passing the value as -D param in mvn command line

@fanf
Copy link
Member Author

fanf commented Sep 12, 2017

PR rebased

@fanf fanf force-pushed the ust_11035/add_the_possibility_to_enable_disable_the_plugin_globally branch from f95af49 to 077b01f Compare September 12, 2017 12:21
@fanf
Copy link
Member Author

fanf commented Sep 12, 2017

PR rebased

@fanf fanf force-pushed the ust_11035/add_the_possibility_to_enable_disable_the_plugin_globally branch from 077b01f to 742ed49 Compare September 12, 2017 12:23
@Normation-Quality-Assistant

OK, merging this PR

@Normation-Quality-Assistant Normation-Quality-Assistant merged commit 742ed49 into Normation:branches/rudder/4.1 Sep 13, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants