Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix ProviderConfig setTimeout generic error #1143

Merged
merged 2 commits into from
Dec 14, 2021

Conversation

wind-hx
Copy link
Contributor

@wind-hx wind-hx commented Dec 4, 2021

Motivation:

The result of calling setTimeout has no generic

@sofastack-bot
Copy link

sofastack-bot bot commented Dec 4, 2021

Hi @wind-hx, welcome to SOFAStack community, Please sign Contributor License Agreement!

After you signed CLA, we will automatically sync the status of this pull request in 3 minutes.

@sofastack-bot sofastack-bot bot added bug Something isn't working cla:no Need sign CLA First-time contributor First-time contributor size/XS labels Dec 4, 2021
@JervyShi JervyShi added this to the 5.8.2 milestone Dec 4, 2021
@JervyShi
Copy link
Member

JervyShi commented Dec 4, 2021

@wind-hx Could you help fix similar problems in ConsumerConfig?

@sofastack-bot sofastack-bot bot added cla:yes CLA is ok and removed cla:no Need sign CLA labels Dec 4, 2021
@wind-hx
Copy link
Contributor Author

wind-hx commented Dec 4, 2021

@wind-hx Could you help fix similar problems in ConsumerConfig?

bug fixes

@codecov
Copy link

codecov bot commented Dec 5, 2021

Codecov Report

Merging #1143 (540e52c) into master (303ae5e) will decrease coverage by 0.11%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1143      +/-   ##
============================================
- Coverage     71.07%   70.96%   -0.12%     
+ Complexity      833      828       -5     
============================================
  Files           405      405              
  Lines         17100    17108       +8     
  Branches       2669     2670       +1     
============================================
- Hits          12154    12140      -14     
- Misses         3561     3588      +27     
+ Partials       1385     1380       -5     
Impacted Files Coverage Δ
...ava/com/alipay/sofa/rpc/config/ConsumerConfig.java 77.64% <ø> (ø)
...ava/com/alipay/sofa/rpc/config/ProviderConfig.java 54.00% <ø> (ø)
...java/com/alipay/sofa/rpc/module/LookoutModule.java 52.38% <0.00%> (-33.34%) ⬇️
...fa/rpc/transport/http/SyncInvokeClientHandler.java 51.85% <0.00%> (-33.34%) ⬇️
.../rpc/transport/http/Http2ClientChannelHandler.java 60.52% <0.00%> (-15.79%) ⬇️
...ipay/sofa/rpc/message/http/HttpResponseFuture.java 46.66% <0.00%> (-4.45%) ⬇️
.../main/java/com/alipay/sofa/rpc/event/EventBus.java 80.39% <0.00%> (-3.93%) ⬇️
...c/transport/http/AbstractHttp2ClientTransport.java 76.64% <0.00%> (-2.40%) ⬇️
.../alipay/sofa/rpc/metrics/lookout/RpcLookoutId.java 87.30% <0.00%> (-1.59%) ⬇️
...om/alipay/sofa/rpc/metrics/lookout/RpcLookout.java 80.00% <0.00%> (-0.96%) ⬇️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 303ae5e...540e52c. Read the comment docs.

@EvenLjj EvenLjj linked an issue Dec 5, 2021 that may be closed by this pull request
Copy link
Collaborator

@EvenLjj EvenLjj left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@JervyShi JervyShi left a comment

Choose a reason for hiding this comment

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

LGTM

@seeflood
Copy link
Member

2021-12-08 14:05:40,432 main ERROR [com.alipay.sofa.rpc.server.triple.TripleServer:error:187] - Register triple service error
java.lang.IllegalStateException: Can not expose service with same name:helloworld.Greeter

The error message in the integration tests is weird.
This helloworld.Greeter was not modified in this PR.
@EvenLjj @OrezzerO Could you help check this problem?

@EvenLjj
Copy link
Collaborator

EvenLjj commented Dec 12, 2021

2021-12-08 14:05:40,432 main ERROR [com.alipay.sofa.rpc.server.triple.TripleServer:error:187] - Register triple service error
java.lang.IllegalStateException: Can not expose service with same name:helloworld.Greeter

The error message in the integration tests is weird. This helloworld.Greeter was not modified in this PR. @EvenLjj @OrezzerO Could you help check this problem?

I will check this problem.

@EvenLjj
Copy link
Collaborator

EvenLjj commented Dec 14, 2021

@wind-hx @seeflood There were some problems in the previous unit test, and i will post a pr to solve this problem. This will be merged first, and finally thank you for participating.

@EvenLjj EvenLjj merged commit a38e4b3 into sofastack:master Dec 14, 2021
@wind-hx wind-hx deleted the fix_ProviderConfig_setTimeout branch December 15, 2021 08:01
@EvenLjj
Copy link
Collaborator

EvenLjj commented Jan 5, 2022

@wind-hx
The way to set ProviderConfig in method setRef will throw the exception capture of ? after this PR merge. May have some compatibility problem.

  RegistryConfig<?> config = new RegistryConfig();
  ServerConfig serverConfig = new ServerConfig();
  ProviderConfig<?> testServiceProviderConfig = new ProviderConfig<>();
  testServiceProviderConfig.setInterfaceId("TestService")
          .setUniqueId("unique123Id")
          .setApplication(new ApplicationConfig().setAppName("test-server"))
          .setProxy("javassist")
          .setRegister(true)
          .setRegistry(config)
          .setSerialization("hessian2")
          .setServer(serverConfig)
          .setWeight(222)
          .setTimeout(3000).setRef(new TestServiceImpl());

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working cla:yes CLA is ok First-time contributor First-time contributor size/XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ProviderConfig setTimeout 返回结果范型没了
4 participants