Skip to content

Fix Issue #509 #634

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

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Fix Issue #509 #634

wants to merge 5 commits into from

Conversation

AydanPirani
Copy link

In-progress.

@AydanPirani
Copy link
Author

Hey! @GlenDC @soundofspace Got a very rudimentary implementation going, can I get a quick informal review on this before finishing implementation?

@GlenDC GlenDC requested review from GlenDC and soundofspace July 15, 2025 07:41
@@ -0,0 +1,22 @@
//! This example demonstrates how to use the http curl command
Copy link
Member

Choose a reason for hiding this comment

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

this file is to be deleted as discussed in the parent isuse

Copy link
Author

Choose a reason for hiding this comment

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

Yup, using this as a temporary driver. Will delete before finishing up.

Copy link
Member

@GlenDC GlenDC left a comment

Choose a reason for hiding this comment

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

Looks like a good start. Next to my comments and fact that you also want to add a good test suite for this (e.g. as curl/tests.rs) I also would say:

  • do not forget about checking the extension ProxyAddress (which defines if you need to set a proxy)
  • there might be some other stuff but cannot think of it now
  • once your finished with code + tests, also add it to the EchoService core code please

@soundofspace might have some more feedback as well.

Either way, well done. Definitely well on your way in resolving this.

@soundofspace soundofspace linked an issue Jul 16, 2025 that may be closed by this pull request
@AydanPirani
Copy link
Author

@GlenDC @soundofspace Hey hey! Running into a small issue here: how do I get the body data to convert to Bytes? Can't seem to figure this one out, any pointers would be much appreciated :)

@GlenDC
Copy link
Member

GlenDC commented Jul 18, 2025

@GlenDC @soundofspace Hey hey! Running into a small issue here: how do I get the body data to convert to Bytes? Can't seem to figure this one out, any pointers would be much appreciated :)

👇

let body = body.collect().await.unwrap().to_bytes();

@AydanPirani
Copy link
Author

@GlenDC Got it, thanks. Also, do you have any to check if a Proxy is enabled?

@GlenDC
Copy link
Member

GlenDC commented Jul 18, 2025

@GlenDC Got it, thanks. Also, do you have any to check if a Proxy is enabled?

Probably gonna change a tiny bit in near future but for now you can check if ProxyAddress (https://ramaproxy.org/docs/rama/net/address/struct.ProxyAddress.html) is present in the Context: if let Some(addr) = ctx.get::<ProxyAddress>() { '-x {addr}' }

something like that in hybrid pseudo code

@AydanPirani
Copy link
Author

Got it, thanks. And one last thing - is a string a valid body type? I presume not, in that case is this the right approach to testing with a stream?

@GlenDC
Copy link
Member

GlenDC commented Jul 18, 2025

Got it, thanks. And one last thing - is a string a valid body type? I presume not, in that case is this the right approach to testing with a stream?

You are confusing different kinds of echo services. You are linking to a stream echo service. That lives on the transport layer. That's different from the echo service which I was linking you that works with http on the application layer.

The first lives on the transport layer (e.g. tcp) and just echo's back the incoming bytes as-is. You can as a human easily play with it using tooling like telnet. There's a tcp example about it that you can try out.

But this is nothing to do with your work here. Different kind of echo service.

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.

add curl module to rama-http-types
3 participants