Skip to content

feat: support dubbo#3224

Merged
spacewander merged 4 commits into
apache:masterfrom
spacewander:dubbo
Jan 20, 2021
Merged

feat: support dubbo#3224
spacewander merged 4 commits into
apache:masterfrom
spacewander:dubbo

Conversation

@spacewander

Copy link
Copy Markdown
Member

Signed-off-by: spacewander [email protected]

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 force-pushed the dubbo branch 10 times, most recently from 1d991cb to 95b84d6 Compare January 12, 2021 03:59
@spacewander spacewander changed the title WIP: support dubbo feat: support dubbo Jan 12, 2021
@spacewander
spacewander force-pushed the dubbo branch 19 times, most recently from 19b6262 to 79596f6 Compare January 12, 2021 11:48
@spacewander
spacewander force-pushed the dubbo branch 4 times, most recently from bd1223e to fc4dfc3 Compare January 12, 2021 13:35
@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
@moonming
moonming requested review from membphis and nic-chen January 18, 2021 02:42
Fix apache#89

Signed-off-by: spacewander <[email protected]>
Comment thread doc/plugins/dubbo-proxy.md
<dubbo:application name="demo-provider"/>

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

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.

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

Copy link
Copy Markdown
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 <[email protected]>

@membphis membphis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Comment thread doc/plugins/dubbo-proxy.md Outdated

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

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

Comment thread README.md Outdated
- **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
Copy Markdown
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?

Comment thread t/plugin/dubbo-proxy/route.t Outdated



=== TEST 5: work with comsumer

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.

Typo: consumer.

Copy link
Copy Markdown
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 <[email protected]>
Signed-off-by: spacewander <[email protected]>
@spacewander
spacewander merged commit e3601e6 into apache:master Jan 20, 2021
@spacewander
spacewander deleted the dubbo branch January 20, 2021 01:47
@wxbty

wxbty commented Jul 21, 2023

Copy link
Copy Markdown
Member

@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