Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Channel proxies direct binding #113

Closed
wants to merge 14 commits into from
Closed

Channel proxies direct binding #113

wants to merge 14 commits into from

Conversation

mbogoevici
Copy link
Contributor

  • Remove DirectChannelProxyFactory;
  • Proxies create channels as necessary, including support for PollableChannels;
  • Channel beans are using the bean factory methods for the proxies;
  • Coordinate bind/unbind across a context through the internal Bindable interface;
  • Add support for embedding modules as independent child contexts of the main module, wrapped in a Bindable interface;
  • Add support for channel name namespacing;
  • Use a shared channel registry (consulted by the proxies) to create direct bindings;

Test by installing spring-cloud-stream and spring-cloud-stream-modules and then running similar to

java -jar spring-cloud-stream-module-launcher/target/spring-cloud-stream-module-launcher-1.0.0.BUILD-SNAPSHOT.jar --modules=org.springframework.cloud.stream.module:time-source:1.0.0.BUILD-SNAPSHOT,org.springframework.cloud.stream.module:filter-processor:1.0.0.BUILD-SNAPSHOT,org.springframework.cloud.stream.module:filter-processor:1.0.0.BUILD-SNAPSHOT --args.0.fixedDelay=7 --args.1.expression='payload.contains("6")' --aggregate=true --spring.cloud.stream.bindings.output=filtered

Note that the target queue should be 'filtered' and that the module parameters are applied correctly

ilayaperumalg
ilayaperumalg reviewed Aug 28, 2015
View reviewed changes
...her/src/main/java/org/springframework/cloud/stream/module/launcher/ModuleLauncherRunner.java
@@ -71,4 +71,15 @@ public void run(String... args) throws Exception {
}
return requests;
}

private String[] filterSpringProperties(String[] args) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This one is yet to be used ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops.

@mbogoevici
Copy link
Contributor Author

Pushed a few changes, removing BindableContextWrapper mainly. I think we want to revisit the idea once we get to embedding contexts - I believe that there's something compelling about having an entire context behave like a Bindable component with inputs and outputs, but currently this just complicates things.

ericbottard
ericbottard reviewed Aug 30, 2015
View reviewed changes
...d-stream/src/main/java/org/springframework/cloud/stream/binding/ChannelBindingLifecycle.java

@Override
public void start() {
if (!running) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think running needs to be volatile for this to be safe

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops.

Copy link
Contributor

Choose a reason for hiding this comment

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

or a final AtomicBoolean

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, it's volatile already.

markfisher
markfisher reviewed Aug 31, 2015
View reviewed changes
...-launcher/src/main/java/org/springframework/cloud/stream/module/launcher/ModuleLauncher.java
for (String mainClass : mainClassNames) {
mainClasses.add(ClassUtils.forName(mainClass, classLoader));
}
Runnable moduleAggregatorRunner = new Runnable() {
Copy link
Contributor

Choose a reason for hiding this comment

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

factoring the Runnable to an inner class would improve readability

@mbogoevici
Refactor channel proxy behaviour and add support for direct binding
4ca10ab 
- Remove DirectChannelProxyFactory;
- Proxies create channels as necessary, including support for PollableChannels;
- Channel beans are using the bean factory methods for the proxies;
- Coordinate bind/unbind across a context through the internal Bindable interface;
- Add support for embedding modules as independent child contexts of the main module, wrapped in a Bindable interface;
- Add support for channel name namespacing;
- Use a shared channel registry (consulted by the proxies) to create direct bindings;
@mbogoevici
Restrict the use of SharedChannelRegistry to aggregation
30f5f07
@mbogoevici
Fixed copyrights
093f0d0
@mbogoevici
Polishing based on comments from the previous branch
d6fb2e9
@mbogoevici
Tweaks
6d2364b 
- namespacing takes into account module index, so multiple modules with the same class can partiticipate
- fix how properties are passes to parent/child
- Split ChannelBindingLifecycle from ChannelBindingAdapter and register it with each child;
@mbogoevici
Javadoc updates
50ecf22
@mbogoevici
Pass only prefixed properties
b086780
@mbogoevici
Actually use the filtering method
9d12cda
@mbogoevici
Lifecycle simplifications
04494ca 
- Removing BindableChannelWrapper and UnbindOnCloseApplicationListener and relying on ChannelBindingListener solely;
- Some code cleanup

Further simplifications
@mbogoevici
More cleanup
8023d02
@mbogoevici
Remove BindableUtils, rename ChannelBindingAdapter
45232b0
@mbogoevici
Minor improvements
42432f4
@mbogoevici
Addressing some comments, renaming, etc
bfff958
@mbogoevici
More polishing
ace6551
@mbogoevici
Copy link
Contributor Author

Currently, the following possible improvements have been identified, to be addressed as part of future work:

  • splitting BindableProxyFactory;
  • Elliminating SharedChannelRegistry and relying on the shared channels being registered directly;
  • Splitting the parent context and launching modules as independent contexts - allowing for more gradual control of lifecycle, binding, startup/shutdown order;

@markfisher
Copy link
Contributor

rebased, squashed and merged as bd47560

@markfisher markfisher closed this Sep 2, 2015
@markfisher markfisher self-assigned this Sep 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@markfisher markfisher

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mbogoevici @markfisher @ilayaperumalg @ericbottard