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.

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

[feat]: add new samples java interop - service discovery #727

Merged
merged 19 commits into from
Mar 27, 2024

Conversation

Yoda-wu
Copy link
Contributor

@Yoda-wu Yoda-wu commented Mar 15, 2024

i finish the samples with java-go interop service discovery both service level and interface level
related issue: #698

Copy link
Contributor

@DMwangnima DMwangnima left a comment

Choose a reason for hiding this comment

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

Nice job! BTW, pls fix the CI problem and add this test case to CI.


import (
"context"
// "time"
Copy link
Contributor

Choose a reason for hiding this comment

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

If you do not need these packages, pls remove them

"dubbo.apache.org/dubbo-go/v3"
// "dubbo.apache.org/dubbo-go/v3/config"

_ "dubbo.apache.org/dubbo-go/v3/client"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it implicitly imported here?

registry.WithAddress("127.0.0.1:8848"),
),

// dubbo.WithMetadataReport(metadata.WithMetadata())),
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto.

logger.Error(err)
}
logger.Infof("Greet response: %s", resp)
// select {}
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto.

"context"

"dubbo.apache.org/dubbo-go/v3"
// "dubbo.apache.org/dubbo-go/v3/config"
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto.

return resp, nil
}

func (s *GreetTripleServer) MethodMapper() map[string]string {
Copy link
Contributor

Choose a reason for hiding this comment

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

If we do not add MethodMapper method, Java client could not invoke the Go Server?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, it turn out that java client can still invoke the go server. i made a mistake. The reason why i add MethodMapper is https://cn.dubbo.apache.org/zh-cn/overview/mannual/golang-sdk/tutorial/develop/interflow/call_java/#22-java-client---go-server
and i referenced the wrong part (Part2 Hessian2 serialize)

@DMwangnima
Copy link
Contributor

For dubbo java part, cc @FoghostCn .

Copy link
Member

@justxuewei justxuewei left a comment

Choose a reason for hiding this comment

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

Please check your code:

  • Comments start with a space, for example:
//Comment goes here (bad case
// Comment goes here (good case)
  • Remove unused code instead of leaving it commented out.

@justxuewei
Copy link
Member

justxuewei commented Mar 17, 2024

Btw, I'd like to suggest you formatting your java code, e.g. google-java-format. What formatting tool is used in Dubbo? /cc @chickenlj

@Yoda-wu
Copy link
Contributor Author

Yoda-wu commented Mar 17, 2024

i will fix it soon

@FoghostCn
Copy link
Contributor

The generated code does not need to be submitted, just configure the maven idl plugin

import java.util.concurrent.CountDownLatch;

public class Main {
private static GreetService greetService;
Copy link
Contributor

Choose a reason for hiding this comment

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

pls make greetService as method private

<modelVersion>4.0.0</modelVersion>

<groupId>org.apache.dubbo</groupId>
<artifactId>samples</artifactId>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use a more meaningful module name

<modelVersion>4.0.0</modelVersion>

<groupId>org.apache.dubbo</groupId>
<artifactId>samples</artifactId>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use a more meaningful module name

@Yoda-wu
Copy link
Contributor Author

Yoda-wu commented Mar 17, 2024

ok, i will fix it soon

@FoghostCn
Copy link
Contributor

Btw, I'd like to suggest you formatting your java code, e.g. google-java-format. What formatting tool is used in Dubbo? /cc @chickenlj

spotless plugin is used in dubbo-java

mvn spotless:apply

@chickenlj chickenlj merged commit 8976c91 into apache:main Mar 27, 2024
2 checks passed
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.

None yet

5 participants