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: support dubbo #3224

Merged
merged 4 commits into from
Jan 20, 2021
Merged

feat: support dubbo #3224

merged 4 commits into from
Jan 20, 2021

Conversation

spacewander
Copy link
Member

Signed-off-by: spacewander spacewanderlzx@gmail.com

What this PR does / why we need it:

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@spacewander spacewander changed the title WIP: support dubbo feat: support dubbo Jan 12, 2021
@spacewander spacewander marked this pull request as ready for review January 12, 2021 14:01
@membphis membphis added this to the 2.3 milestone Jan 13, 2021
Fix apache#89

Signed-off-by: spacewander <spacewanderlzx@gmail.com>
doc/plugins/dubbo-proxy.md Show resolved Hide resolved
<dubbo:application name="demo-provider"/>

<!-- use multicast registry center to export service -->
<dubbo:registry address="multicast://224.5.6.7:1234"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Multicast is for test and develop environment, not for the production.

Copy link
Member Author

Choose a reason for hiding this comment

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

This part of code is just provided a test backend, it is not used for prod.

Signed-off-by: spacewander <spacewanderlzx@gmail.com>
Copy link
Member

@membphis membphis left a comment

Choose a reason for hiding this comment

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

LGTM


curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
{
"methods": ["GET"],
Copy link
Member

Choose a reason for hiding this comment

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

user example, less is better. so we can remove this line

@membphis membphis requested a review from tokers January 18, 2021 09:32
@membphis
Copy link
Member

We need a way to avoid recompiling openresty every time in ci. We can implement this in a new PR.

Now it takes 20 minutes, which is a bit long.

image

cd t/lib/dubbo-backend
mvn package
cd dubbo-backend-provider/target
java -Djava.net.preferIPv4Stack=true -jar dubbo-demo-provider.one-jar.jar > /tmp/java.log &
Copy link
Member

Choose a reason for hiding this comment

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

LGTM

It is better to move t/lib/dubbo-backend to a separate repo, and only need to download the compiled jar when running CI.

Copy link
Member Author

Choose a reason for hiding this comment

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

As the content of Java code is depended on what you want to test, it is inconvenient to move it to a separate repo.

Copy link
Member

Choose a reason for hiding this comment

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

mvn package + java -Djava.net.preferIPv4Stack=true

How long does it usually take?

Copy link
Member Author

Choose a reason for hiding this comment

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

@membphis
32 seconds

Copy link
Member

Choose a reason for hiding this comment

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

I think this is acceptable

README.md Outdated
@@ -79,6 +79,7 @@ A/B testing, canary release, blue-green deployment, limit rate, defense against
- **Multi protocols**

- [TCP/UDP Proxy](doc/stream-proxy.md): Dynamic TCP/UDP proxy.
- [dubbo Proxy](doc/plugins/dubbo-proxy.md): Dynamic HTTP to dubbo proxy.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe Dubbo is better?




=== TEST 5: work with comsumer
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: consumer.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

Signed-off-by: spacewander <spacewanderlzx@gmail.com>
Signed-off-by: spacewander <spacewanderlzx@gmail.com>
@spacewander spacewander merged commit e3601e6 into apache:master Jan 20, 2021
@spacewander spacewander deleted the dubbo branch January 20, 2021 01:47
@wxbty
Copy link
Member

wxbty commented Jul 21, 2023

@spacewander Hi, can you please reply, will the dubbo backend service start automatically when running the test? I opened a PR #9660 that supports the dubbo protocol, and want to reuse the dubbo backend service, but by default, connection refused is returned.

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.

6 participants