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

tcpのexecを作った #98

Merged
merged 5 commits into from
May 29, 2024
Merged

tcpのexecを作った #98

merged 5 commits into from
May 29, 2024

Conversation

shunsuke-shimomura
Copy link
Contributor

#93 でのレビューを受けて、execでtcpの受け口を作ることにした。
とりあえず作ったけど動かないので皆さん助けてください!!

@kobkaz
Copy link
Contributor

kobkaz commented May 27, 2024

手元で簡単に試してみましたが、ひとまず正常に動いていると思いますよ

@shunsuke-shimomura
Copy link
Contributor Author

shunsuke-shimomura commented May 27, 2024

そうすると自分が試してるTCP通信側の問題ということな気がしてきました。皆さんの確認が終わり次第こっちをマージしてもう片方の方は消します

@shunsuke-shimomura
Copy link
Contributor Author

手元にあったTCPポートが複数接続を禁止してるだけだったので、適当なTCPポートを作ったらうまく動きました!

@shunsuke-shimomura
Copy link
Contributor Author

マージしてもいいですか?

#[tokio::main]
async fn main() -> Result<()> {
let args: Vec<String> = args().collect();
let addr = format!("{}:{}", args.get(1).unwrap(), args.get(2).unwrap());
Copy link
Member

Choose a reason for hiding this comment

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

コマンドライン引数の取り扱いは clap を用いて欲しい

@kobkaz
Copy link
Contributor

kobkaz commented May 27, 2024

すみません まだレビューできてないので
あとどこにもはっきり書かれてはいないのですが、マージボタンを押す係は @KOBA789 がやる方針になっています

clap.workspace = true
eb90 = "0.1.1"
notalawyer.workspace = true
notalawyer-clap.workspace = true
Copy link
Member

Choose a reason for hiding this comment

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

末尾に改行文字が欲しい

tracing.workspace = true
tracing-subscriber.workspace = true
clap.workspace = true
eb90 = "0.1.1"
Copy link
Member

Choose a reason for hiding this comment

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

eb90 は不要では?

tokio-util.workspace = true
bytes.workspace = true
tracing.workspace = true
tracing-subscriber.workspace = true
Copy link
Member

Choose a reason for hiding this comment

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

tracing も活用できてなさそう。deps に含めるなら eprintln ではなく tracing をちゃんと使うべき

@shunsuke-shimomura
Copy link
Contributor Author

ありがとうございます!修正します

Comment on lines 11 to 12
host: String,
port: String,
Copy link
Member

Choose a reason for hiding this comment

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

std::net::SocketAddrFromStr を実装しているので、文字列で受け取らずに直接 SocketAddr で受け取れます。
そうすればそれを TcpStream::connect に直接渡せますし、IPv6 にも対応できます。

Suggested change
host: String,
port: String,
addr: SocketAddr,

Copy link
Member

@KOBA789 KOBA789 left a comment

Choose a reason for hiding this comment

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

コードを読む限りでは大丈夫そう。
動作確認はできていないので、もしどなたか余裕あれば代わりにお願いしたいです(キツそうなら私の方でやります)

@KOBA789
Copy link
Member

KOBA789 commented May 28, 2024

@shunsuke-shimomura (どこにも書いてないんですが)一応コミットメッセージは英語で統一しているので、rebase とかで書き直してもらっていいですか? 書き直したら force push しちゃってよいです。

@kobkaz
Copy link
Contributor

kobkaz commented May 28, 2024

動作確認については

{
  {
  plugs: {
    tcp : "exec:kble-tcp 127.0.0.1 8000"
  },
  links : {
    tcp: "tcp"
  }
}

を用いて、127.0.0.1:8000でlistenしているサーバからクライアントに向けて送信したデータがサーバに返ってくることを確認しています

Copy link
Member

@KOBA789 KOBA789 left a comment

Choose a reason for hiding this comment

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

@shunsuke-shimomura コミットメッセージ直してもらったらマージします

@KOBA789 KOBA789 merged commit 6e86aec into main May 29, 2024
1 check passed
@KOBA789 KOBA789 deleted the add-tcp-exec branch May 29, 2024 04:55
This was referenced Nov 19, 2024
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.

3 participants