Http client support #253
Http client support #253
Conversation
|
At least the basics works:
|
|
Nice idea! Thank for contributing! I'll do a more in-depth review asap |
|
Thanks! I'll keep this in draft status until I get some of the other http verbs implemented but I'd love feedback, especially on other trait functions and dependency/feature related stuff |
|
The http parser doesn't curently implement variable substitution, so we will need to upgrade the dependency version once Laeri/http-rest-file#1 gets merged if we want that feature |
src/interpreters/Http_original.rs
Outdated
| fn execute(&mut self) -> Result<String, SniprunError> { | ||
| let model = http_rest_file::Parser::parse(&self.code, false); | ||
|
|
||
| for req in model.requests.into_iter() { |
There was a problem hiding this comment.
If this is meant to iterate over the several request that may appear in a snippet, it's defeated by the 'return Ok/Err' later on.
There is no way of returning 'several' Ok/Err results so there is two possibilities:
- aggregate yourself in a way that makes sense the different request responses
- Use the
return Err(SniprunError::ReRunRanges(ranges))trick (see the GFM interpreter, for example) to re-launch your interpreter over several snippets (i'd prefer this, unless it'd break due to semantic dependencies between request (variables?) that can't be solved in another way). In this case, have a robust "i have several requests" detection to avoid returning ReRunRanges again !
|
Hi @reavessm , I 'fixed' a modification you made due to an incorrect comment of mine (error_truncate stuff, sorry). Do you agree with the latest commit on the 'http' branch I just pushed ? (it does silently fails if the resp.into_string() errors out, but I don't see that happening in the real-world, or if it does, I hope the status (not-200?) will be indicative enough) |
|
Oh, I like that! Thank you! That really cleans it up. I'll add that to this PR. I'm going to reopen this for review. I also think we shouldn't rerun anything in case somethings fails. I can see a situation where you have three REST calls that you want to execute sequentially and if the second one fails, you don't want the third to run. Especially considering this isn't naming any of the requests, I think sequential order is important |
Fair enough
|
|
@reavessm Are you satisfied with the state of your contribution ? However, I'd like to do my own dev merge and add some cosmetics (update readme and doc) about HTTP support, so I can't give you permission to merge into master anytime you want via the review system. (Because of how most vim plugin manager works, the master branch has to be carefully synchronized with sniprun's binary releases) |
Without this fix it currently becomes
tag_to_fetch='This release and associated artifacts were created by Github Action. See the changelog [here](https://github.com/michaelb/sniprun/blob/master/CHANGELOG.md).'
|
@michaelb Yep! I can change this to be based on the dev branch. And if you need any help with docs I don't mind helping |
it makes the virtualtext display much nicer
Support for testing rest endpoint via sniprun
Following http file syntax