Skip to content
This repository was archived by the owner on Oct 13, 2024. It is now read-only.

[SCB-244] enable servicecomb and add demo#210

Merged
WillemJiang merged 7 commits intoapache:masterfrom
zhengyangyong:SCB-244
Jun 26, 2018
Merged

[SCB-244] enable servicecomb and add demo#210
WillemJiang merged 7 commits intoapache:masterfrom
zhengyangyong:SCB-244

Conversation

@zhengyangyong
Copy link

@zhengyangyong zhengyangyong commented Jun 20, 2018

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a JIRA issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Format the pull request title like [SCB-XXX] Fixes bug in ApproximateQuantiles, where you replace SCB-XXX with the appropriate JIRA issue.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Run mvn clean install to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

zhengyangyong added 6 commits June 19, 2018 16:56
Signed-off-by: zhengyangyong <yangyong.zheng@huawei.com>
Signed-off-by: zhengyangyong <yangyong.zheng@huawei.com>
Signed-off-by: zhengyangyong <yangyong.zheng@huawei.com>
Signed-off-by: zhengyangyong <yangyong.zheng@huawei.com>
Signed-off-by: zhengyangyong <yangyong.zheng@huawei.com>
Signed-off-by: zhengyangyong <yangyong.zheng@huawei.com>
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.8%) to 94.161% when pulling be0e7d0 on zhengyangyong:SCB-244 into d24cb61 on apache:master.

@coveralls
Copy link

coveralls commented Jun 20, 2018

Coverage Status

Coverage decreased (-1.7%) to 93.194% when pulling 5a51819 on zhengyangyong:SCB-244 into d24cb61 on apache:master.

@zhengyangyong
Copy link
Author

This pr need java chassis 1.0.0-m2

Copy link
Member

@WillemJiang WillemJiang left a comment

Choose a reason for hiding this comment

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

Please update the accept test with this demo at the same time.

<artifactId>omega-transport-servicecomb</artifactId>

<properties>
<java-chassis.version>1.0.0-m2</java-chassis.version>
Copy link
Member

Choose a reason for hiding this comment

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

Java chassis version is used in the root pom, we don't need to define it here.

Copy link
Author

Choose a reason for hiding this comment

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

Done

try {
omegaContext = BeanUtils.getBean("omegaContext");
} catch (NullPointerException npe) {
LOG.warn("The OmegaContext is not injected, The SagaConsumerHandler is disabled.");
Copy link
Member

Choose a reason for hiding this comment

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

The warning message need to updated.
SagaConsumerHandler is not disabled, it's just cannot inject transaction ID.

Copy link
Author

Choose a reason for hiding this comment

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

Done

LOCAL_TX_ID_KEY,
omegaContext.localTxId());
} else {
LOG.warn("The OmegaContext is not injected, The SagaConsumerHandler is disabled.");
Copy link
Member

Choose a reason for hiding this comment

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

The SagaConsumerHandler just skip the work, the log level can be info.

Copy link
Author

Choose a reason for hiding this comment

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

Done

omegaContext.setLocalTxId(invocation.getContext().get(LOCAL_TX_ID_KEY));
}
} else {
LOG.warn("The OmegaContext is not injected, The SagaConsumerHandler is disabled.");
Copy link
Member

Choose a reason for hiding this comment

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

The warning need to be updated.

Copy link
Author

Choose a reason for hiding this comment

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

Done

this.omegaContext = omegaContext;
if (omegaContext == null) {
LOG.info("The OmegaContext is not injected, The SagaProviderHander is disabled.");
private OmegaContext omegaContext;
Copy link
Member

Choose a reason for hiding this comment

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

It's better to use the final object to avoid the change from outside.
We could add a new construction method which set the omegaContext to be null.

Copy link
Author

Choose a reason for hiding this comment

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

Done

private final AsyncResponse asyncResponse = mock(AsyncResponse.class);

private final SagaConsumerHandler handler = new SagaConsumerHandler(omegaContext);
private final SagaConsumerHandler handler = new SagaConsumerHandler();
Copy link
Member

Choose a reason for hiding this comment

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

Please keep the old code.

Copy link
Author

Choose a reason for hiding this comment

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

Done

<maven.compiler.source>1.8</maven.compiler.source>
<maven.compiler.target>1.8</maven.compiler.target>
<saga.version>0.3.0-SNAPSHOT</saga.version>
<java-chassis.version>1.0.0-m2</java-chassis.version>
Copy link
Member

Choose a reason for hiding this comment

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

we should inherited the version from root pom.xml

Copy link
Author

Choose a reason for hiding this comment

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

Done

@WillemJiang
Copy link
Member

As the java-chassis 1.0.0-m2 is not released yet, you may need to use 1.0.0-m1 to get the CI passed.

Signed-off-by: zhengyangyong <yangyong.zheng@huawei.com>
context = BeanUtils.getBean("omegaContext");
} catch (NullPointerException npe) {
LOG.warn("The OmegaContext is not injected, The SagaConsumerHandler is disabled.");
LOG.warn("SagaConsumerHandler is not disabled, it's just cannot inject transaction ID.");
Copy link
Member

Choose a reason for hiding this comment

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

The waring message should be "The OmegaContext is null, SagaConsumer cannot inject transaction ID"

@WillemJiang WillemJiang merged commit b450957 into apache:master Jun 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants