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

Modify the documentation about the Telnet command to match the code impl… #200

Closed
wants to merge 1 commit into from

Conversation

kexianjun
Copy link
Member

…ementation

What is the purpose of the change

Modify the documentation of the Telnet command to match the code implementation

Brief changelog

XXXXX

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a GITHUB_issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a GITHUB issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
  • Format the pull request title like Fix UnknownException when host config not exist #XXX. Each commit in the pull request should have a meaningful subject line and body.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Test your code locally by running docsite start, and make sure it works as expected.
  • Make sure no files under build directory is added.
  • If this contribution is large, please follow the Software Donation Guide.

@kexianjun
Copy link
Member Author

@beiwei30 would you mind to review this pr?

@kexianjun kexianjun changed the title Modify the documentation of the Telnet command to match the code impl… Modify the documentation about the Telnet command to match the code impl… Dec 12, 2018
@ralf0131 ralf0131 self-assigned this Dec 12, 2018
@ralf0131 ralf0131 self-requested a review December 12, 2018 03:55
0. `invoke XxxService.xxxMethod({"prop": "value"})`: invoke particular method for the given service
0. `invoke xxxMethod({"prop": "value"})`: invoke particular method for the default service
0. `invoke xxxService.xxxMethod("paramStr")`或`invoke xxxService.xxxMethod(18)`: invoke particular method with a primitive type parameter
0. `invoke xxxService.xxxMethod("paramStr") -p java.lang.String`: invoke particular method with a String type parameter

Choose a reason for hiding this comment

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

I ran the following command on latest 2.7.x branch and got following error

dubbo>invoke org.apache.dubbo.demo.DemoService.sayHello("hello") -p java.lang.String
Invalid parameters, format: service.method(args)

It looks like -p java.lang.String does not work. How did you get it to work?

Copy link
Member Author

@kexianjun kexianjun Dec 21, 2018

Choose a reason for hiding this comment

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

I ran the following command on latest 2.7.x branch and got following error

dubbo>invoke org.apache.dubbo.demo.DemoService.sayHello("hello") -p java.lang.String
Invalid parameters, format: service.method(args)

It looks like -p java.lang.String does not work. How did you get it to work?

I have made more test on the latest master branch.My test is as the following:

package org.apache.dubbo.demo;

public class Student {
    private String name;
    private int age;

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }

    public int getAge() {
        return age;
    }

    public void setAge(int age) {
        this.age = age;
    }
}
package org.apache.dubbo.demo;

public interface DemoService {

    String sayHello(String name);

    String sayHello(Long name);

    String sayHello(Integer age);

    String sayHello(Student student);

    String sayHello(String name, Integer age);

}
package org.apache.dubbo.demo.provider;

import org.apache.dubbo.demo.DemoService;
import org.apache.dubbo.demo.Student;
import org.apache.dubbo.rpc.RpcContext;

import java.text.SimpleDateFormat;
import java.util.Date;

public class DemoServiceImpl implements DemoService {

    @Override
    public String sayHello(String name) {
        System.out.println("[" + new SimpleDateFormat("HH:mm:ss").format(new Date()) + "] Hello " + name + ", request from consumer: " + RpcContext.getContext().getRemoteAddress());
        return "Hello " + name + ", response from provider: " + RpcContext.getContext().getLocalAddress();
    }

    @Override
    public String sayHello(Long name) {
        return "Long from sayHello " + name;
    }

    @Override
    public String sayHello(Integer age) {
        return "int from sayHello " + age;
    }

    @Override
    public String sayHello(Student student) {
        return "Student from sayHello ,name:" + student.getName() + ",age:" + student.getAge();
    }

    @Override
    public String sayHello(String name, Integer age) {
        return "name and age from sayHello,name:" + name + ",age" + age;
    }
}

here is my result:
image
image

And based on your result,the error message from org/apache/dubbo/rpc/protocol/dubbo/telnet/InvokeTelnetHandler.java:158

if (i < 0 || !message.endsWith(")")) {
            return "Invalid parameters, format: service.method(args)";
        }

it may indicate something wrong with the input parentheses or the following class

org/apache/dubbo/rpc/protocol/dubbo/telnet/InvokeTelnetHandler.java

different from the master branch

.Would you maind to take a look at this and provide more detail about your test?

Choose a reason for hiding this comment

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

Hi, sorry for the late reply, I am going to try it out in next few days.

@kexianjun
Copy link
Member Author

close since the -p feature has beed removed in this pr Refactor telnet invoke command

@kexianjun kexianjun closed this Jan 25, 2019
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.

2 participants