Skip to content

[SCB-612]delete useless MicroserviceMetaManager#861

Merged
liubao68 merged 3 commits intoapache:masterfrom
heyile:cse-612
Aug 14, 2018
Merged

[SCB-612]delete useless MicroserviceMetaManager#861
liubao68 merged 3 commits intoapache:masterfrom
heyile:cse-612

Conversation

@heyile
Copy link
Copy Markdown
Contributor

@heyile heyile commented Aug 8, 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.

@coveralls
Copy link
Copy Markdown

coveralls commented Aug 8, 2018

Coverage Status

Coverage decreased (-0.06%) to 86.441% when pulling b3d84db on heyile:cse-612 into 5754914 on apache:master.

public SchemaMeta ensureFindSchemaMeta(String microserviceName, String schemaId) {
MicroserviceMeta microserviceMeta = microserviceMetaManager.ensureFindValue(microserviceName);
if (!RegistryUtils.getMicroservice().getServiceName().equals(microserviceName)) {
LOGGER.error("miroserviceName : {} is different from the default microserviceName :{}",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is an internally used class, will this logic happen? Or can we delete the parameter microserviceName?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this logic will not happen in the normal mode , and I have asked jimin about whether I could delete parameter microserviceName because it's no use now. He suggested me to keep it for the compatibility of the method with
other classes or modules and he advised me to compare the parameter microserviceName with the default right micorserviceName. just make sure the program is running as we expected。


public Collection<SchemaMeta> getAllSchemaMeta(String microserviceName) {
MicroserviceMeta microserviceMeta = microserviceMetaManager.ensureFindValue(microserviceName);
if (!RegistryUtils.getMicroservice().getServiceName().equals(microserviceName)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same as above

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

same as above


MicroserviceMeta microserviceMeta =
microserviceMetaManager.getOrCreateMicroserviceMeta(RegistryUtils.getMicroservice());
MicroserviceMeta microserviceMeta = SCBEngine.getInstance().getProducerMicroMeta();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

getProducerMicroserviceMeta?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok, i see

import mockit.MockUp;
import mockit.Mocked;

import io.vertx.core.buffer.Buffer;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

did not import correct codestyle?

Copy link
Copy Markdown
Contributor Author

@heyile heyile Aug 11, 2018

Choose a reason for hiding this comment

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

I have import a wrong code style...

if (!RegistryUtils.getMicroservice().getServiceName().equals(microserviceName)) {
LOGGER.error("miroserviceName : {} is different from the default microserviceName :{}",
microserviceName,
RegistryUtils.getMicroservice().getServiceName());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

still go on?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I just want to make a warning there, tell the programer that he may have the mistake with the parameter microserviceName,and you should not set the microserviceName any more. I do not want to stop the program, I'm afraid that some other moduls or methods may invoke the method with the wrong microserviceName,so i just want to give a warnning there, tell them the microserviceName is wrong and keep the program running as usual.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

but the go on logic, will make producerMicroserviceMeta incorrect.

if (!RegistryUtils.getMicroservice().getServiceName().equals(microserviceName)) {
LOGGER.error("miroserviceName : {} is different from the default microserviceName :{}",
microserviceName,
RegistryUtils.getMicroservice().getServiceName());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

still go on?

if (!RegistryUtils.getMicroservice().getServiceName().equals(microserviceName)) {
LOGGER.error("miroserviceName : {} is different from the default microserviceName :{}",
microserviceName,
RegistryUtils.getMicroservice().getServiceName());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

still go on?

if (!RegistryUtils.getMicroservice().getServiceName().equals(microserviceName)) {
LOGGER.error("miroserviceName : {} is different from the default microserviceName :{}",
microserviceName,
RegistryUtils.getMicroservice().getServiceName());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

......

@SuppressWarnings("unchecked")
@Mock
<T> T getBean(String name) {
@Mock <T> T getBean(String name) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why change format?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don not know, when I use
@mock
T getBean(String name) {}
it can not pass the travis ci's check

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

impossible

});
if (wait) {
LOGGER.info("Refreshing remote config...");
LOGGER.debug("Refreshing remote config...");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

who do this change?

@heyile heyile closed this Aug 13, 2018
@heyile heyile reopened this Aug 13, 2018
@heyile heyile closed this Aug 13, 2018
@heyile heyile reopened this Aug 13, 2018
@heyile heyile closed this Aug 13, 2018
@heyile heyile reopened this Aug 13, 2018
@heyile heyile closed this Aug 14, 2018
@heyile heyile reopened this Aug 14, 2018
@heyile heyile closed this Aug 14, 2018
@heyile heyile reopened this Aug 14, 2018
@heyile heyile closed this Aug 14, 2018
@heyile heyile reopened this Aug 14, 2018

public SchemaMeta ensureFindSchemaMeta(String microserviceName, String schemaId) {
MicroserviceMeta microserviceMeta = microserviceMetaManager.ensureFindValue(microserviceName);
if (!RegistryUtils.getMicroservice().getServiceName().equals(microserviceName)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if throw exception, then just remove "microserviceName" parameter is better.
so:
1.just log and return
2.deprecated the method, and invoke another method without parameter "microserviceName"
(old one will be deleted in future version)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

so i would better to choose 2 ?
ok. let me have a try

@wujimin
Copy link
Copy Markdown
Contributor

wujimin commented Aug 14, 2018

too many commit log......

// 动态注册schemas目录下面的契约到当前服务
org.apache.servicecomb.core.definition.loader.DynamicSchemaLoader.INSTANCE.registerSchemas("appServer:appService",
"classpath*:schemas/*.yaml");
org.apache.servicecomb.core.definition.loader.DynamicSchemaLoader.INSTANCE
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remote class CrossappBootListener
because it's used to register target microservice schema.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

so , what should i do there ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sorry, "remove", not "remote"

@heyile heyile closed this Aug 14, 2018
@heyile heyile reopened this Aug 14, 2018
@heyile heyile closed this Aug 14, 2018
@heyile heyile reopened this Aug 14, 2018
@heyile heyile closed this Aug 14, 2018
@heyile heyile reopened this Aug 14, 2018
@heyile heyile closed this Aug 14, 2018
@heyile heyile reopened this Aug 14, 2018
@heyile heyile closed this Aug 14, 2018
@heyile heyile reopened this Aug 14, 2018
@liubao68 liubao68 closed this Aug 14, 2018
@liubao68 liubao68 reopened this Aug 14, 2018
@liubao68 liubao68 merged commit 25d5c45 into apache:master Aug 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants