You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iota.apache.org by cristofolini <gi...@git.apache.org> on 2017/07/06 17:17:31 UTC
[GitHub] incubator-iota pull request #35: IOTA 41: Spring dependency injection for fe...
GitHub user cristofolini opened a pull request:
https://github.com/apache/incubator-iota/pull/35
IOTA 41: Spring dependency injection for fey performers
Our proposed implementation consists of creating a class that extends the original FeyGenericActor (named FeyGenericSpringActor), which allows for one more argument to be passed within its constructor, which is a String containing a path for the XML file containing that actor's Spring context, which can be an absolute or relative filesystem path (relative to the current VM working directory) or obtained from the classpath (by specifying a "classpath:" prefix); other protocols and prefixes may be supported by the Spring framework. The new actor class has two attributes: an ApplicationContext and its AutowireCapableBeanFactory, which will allow us to instantiate the beans specified in the actor's context.
Other modifications to the fey engine core include the necessary changes related to parsing the orchestration JSON: adding an entry for the actor context's file path on the JSON validator and parser method, along with a conditional check to see if a path to the actor's context was specified in the JSON, so that fey will instantiate a FeyGenericSpringActor instead of a FeyGenericActor.
Lastly, we've created a FeyApplicationContext.xml, which should be the parent context to all actor contexts created, and will contain useful universal declarations. For now, all it does is enable annotation usage in all Spring actors.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cristofolini/incubator-iota master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-iota/pull/35.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #35
----
commit 8c1a715090b4b8eb357b5b0bd095a12a1c03f771
Author: Lucas Berri Cristofolini <lu...@autonomiccs.com.br>
Date: 2017-07-06T17:10:07Z
Added a new actor type capable of using the Spring framework
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-iota pull request #35: IOTA-41: Spring dependency injection for fe...
Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on a diff in the pull request:
https://github.com/apache/incubator-iota/pull/35#discussion_r126550150
--- Diff: fey-core/src/main/scala/org/apache/iota/fey/FeyGenericSpringActor.scala ---
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.iota.fey
+
+import akka.actor.ActorRef
+import org.springframework.beans.factory.config.AutowireCapableBeanFactory
+
+import scala.concurrent.duration._
+import org.springframework.context.ApplicationContext
+import org.springframework.context.support.ClassPathXmlApplicationContext
+
+abstract class FeyGenericSpringActor(override val params: Map[String,String] = Map.empty,
+ override val backoff: FiniteDuration = 1.minutes,
+ override val connectTo: Map[String,ActorRef] = Map.empty,
+ override val schedulerTimeInterval: FiniteDuration = 2.seconds,
+ override val orchestrationName: String = "",
+ override val orchestrationID: String = "",
+ override val autoScale: Boolean = false,
+ val appContextPath: String = "") extends FeyGenericActor {
+
+ val appContext: ApplicationContext = new ClassPathXmlApplicationContext()(appContextPath)
+ val factory: AutowireCapableBeanFactory = appContext.getAutowireCapableBeanFactory
+
+ override def onStart(): Unit = {
+ super.onStart()
+ factory.autowireBeanProperties(this, AutowireCapableBeanFactory.AUTOWIRE_BY_NAME, true)
--- End diff --
By name here will force us to use the @Qualifier, right?
What about using by type?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-iota issue #35: IOTA-41: Spring dependency injection for fey perfo...
Posted by barbaragomes <gi...@git.apache.org>.
Github user barbaragomes commented on the issue:
https://github.com/apache/incubator-iota/pull/35
@cristofolini Could you write some unit tests related to this PR?
It would be nice to have some tests that uses the Spring framework.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-iota issue #35: IOTA-41: Spring dependency injection for fey perfo...
Posted by cristofolini <gi...@git.apache.org>.
Github user cristofolini commented on the issue:
https://github.com/apache/incubator-iota/pull/35
@barbaragomes I've added a couple unit tests, one to check that a context is loaded and one to check that a factory is instatiated. I want to get some feedback on two things:
- Am I doing this right? I'm completely new to Scala and these are the first tests I ever coded (in Scala, anyway) and mostly based myself on the other tests that are implemented;
- Anything else that needs testing? Let me know if there's anything else I should cover.
@rafaelweingartner I've changed the autowiring method to by_type.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-iota issue #35: IOTA-41: Spring dependency injection for fey perfo...
Posted by cristofolini <gi...@git.apache.org>.
Github user cristofolini commented on the issue:
https://github.com/apache/incubator-iota/pull/35
@rafaelweingartner All it does right now is assert that an ApplicationContext and a BeanFactory are created when a proper contextPath is present.
I agree that we should have tests for when it isn't. I can see about writing those as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-iota pull request #35: IOTA-41: Spring dependency injection for fe...
Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on a diff in the pull request:
https://github.com/apache/incubator-iota/pull/35#discussion_r126560025
--- Diff: fey-core/src/main/scala/org/apache/iota/fey/FeyGenericSpringActor.scala ---
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.iota.fey
+
+import akka.actor.ActorRef
+import org.springframework.beans.factory.config.AutowireCapableBeanFactory
+
+import scala.concurrent.duration._
+import org.springframework.context.ApplicationContext
+import org.springframework.context.support.ClassPathXmlApplicationContext
+
+abstract class FeyGenericSpringActor(override val params: Map[String,String] = Map.empty,
+ override val backoff: FiniteDuration = 1.minutes,
+ override val connectTo: Map[String,ActorRef] = Map.empty,
+ override val schedulerTimeInterval: FiniteDuration = 2.seconds,
+ override val orchestrationName: String = "",
+ override val orchestrationID: String = "",
+ override val autoScale: Boolean = false,
+ val appContextPath: String = "") extends FeyGenericActor {
+
+ val appContext: ApplicationContext = new ClassPathXmlApplicationContext()(appContextPath)
+ val factory: AutowireCapableBeanFactory = appContext.getAutowireCapableBeanFactory
+
+ override def onStart(): Unit = {
+ super.onStart()
+ factory.autowireBeanProperties(this, AutowireCapableBeanFactory.AUTOWIRE_BY_NAME, true)
--- End diff --
I do find the by_type better, because then we do not need to care about variable names and object names/ids.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-iota pull request #35: IOTA-41: Spring dependency injection for fe...
Posted by cristofolini <gi...@git.apache.org>.
Github user cristofolini commented on a diff in the pull request:
https://github.com/apache/incubator-iota/pull/35#discussion_r126557616
--- Diff: fey-core/src/main/scala/org/apache/iota/fey/FeyGenericSpringActor.scala ---
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.iota.fey
+
+import akka.actor.ActorRef
+import org.springframework.beans.factory.config.AutowireCapableBeanFactory
+
+import scala.concurrent.duration._
+import org.springframework.context.ApplicationContext
+import org.springframework.context.support.ClassPathXmlApplicationContext
+
+abstract class FeyGenericSpringActor(override val params: Map[String,String] = Map.empty,
+ override val backoff: FiniteDuration = 1.minutes,
+ override val connectTo: Map[String,ActorRef] = Map.empty,
+ override val schedulerTimeInterval: FiniteDuration = 2.seconds,
+ override val orchestrationName: String = "",
+ override val orchestrationID: String = "",
+ override val autoScale: Boolean = false,
+ val appContextPath: String = "") extends FeyGenericActor {
+
+ val appContext: ApplicationContext = new ClassPathXmlApplicationContext()(appContextPath)
+ val factory: AutowireCapableBeanFactory = appContext.getAutowireCapableBeanFactory
+
+ override def onStart(): Unit = {
+ super.onStart()
+ factory.autowireBeanProperties(this, AutowireCapableBeanFactory.AUTOWIRE_BY_NAME, true)
--- End diff --
You'd only need a @Qualifier if the bean id in the XML and the autowired attribute's name are different.
The difference between by_name and by_type is exactly what it says on the tin. By_name will autowire beans with an id that matches the name, while by_type will look for a bean of the same type. I don't really have strong reasons to go either way. Do you think it might be better to go by_type?
If so I can change that when I get the tests in.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-iota issue #35: IOTA-41: Spring dependency injection for fey perfo...
Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the issue:
https://github.com/apache/incubator-iota/pull/35
Just to update everyone here.
This PR passed in out simple test cases scenarios, but when we started using it in a production system, it did not work properly. There is a problem on how the class loaders of different agents are being managed. We are working to fix it and then add it here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-iota issue #35: IOTA-41: Spring dependency injection for fey perfo...
Posted by cristofolini <gi...@git.apache.org>.
Github user cristofolini commented on the issue:
https://github.com/apache/incubator-iota/pull/35
@rafaelweingartner I've changed the PR title to include the "-" and now the JIRA ticket is linking to it.
I think an example with Spring actors is a pretty good idea. Let me think something up.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-iota issue #35: IOTA 41: Spring dependency injection for fey perfo...
Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the issue:
https://github.com/apache/incubator-iota/pull/35
@cristofolini can you add IOTA-41 in the PR description? Because of the missing "-" Jira is not linking this PR to the ticket.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---