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

Configure --max-time per request #3162

Open
jcamiel opened this issue Aug 20, 2024 · 4 comments
Open

Configure --max-time per request #3162

jcamiel opened this issue Aug 20, 2024 · 4 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@jcamiel
Copy link
Collaborator

jcamiel commented Aug 20, 2024

Make --max-time configurable per request:

GET https://foo.bin
[Options]
max-time: 500s
HTTP 200
@jcamiel jcamiel added enhancement New feature or request good first issue Good for newcomers labels Aug 20, 2024
@lambrospetrou
Copy link

This would be nice to have, but can we make the CLI option to be the "max" time that any inline option might specify?

I am parsing the Hurl file with hurlfmt and do validation on my side as well, but just want to see how the inline option would interact with the CLI option.

@jcamiel
Copy link
Collaborator Author

jcamiel commented Aug 21, 2024

Ha, didn't see that this can be problematic with https://skybear.net! Honestly, when reviewing the change log for 5.0.0, I was surprised that we haven't implemented it yet and hesitated to label it as a bug.

Currently, option in [Options] section overrides CLI option so it would be strange to do a particular case for this one. And we have some options that can't be configured by request : --file-root, --to-entry. For these options, it makes no sens to be per request configurable. On the other side, a maximum timeout seems a reasonable option per request (I was 100% sure we could do it).

Either we change the priority order of CLI option vs [Options] (I'm not really fan of changing it as I prefer the current priority order), or we don't implement max-time per request, which is unfortunate.

Could you analyse the Hurl file and whitelist only certain option in [Options] section? I can see this being sufficient. Besides --max-time, we have current option that could be problematic if overridden by user like --proxy no?

@lambrospetrou
Copy link

lambrospetrou commented Aug 21, 2024

Could you analyse the Hurl file and whitelist only certain option in [Options] section?

Yes, I do this right now, by rejecting specific options and values. OK, if the precedence priority is for the inline options to override the CLI then I will keep expanding that rejection list on my side.

(I remember the priority discussion in the issue about variables as well and that would be a bit different order, but maybe I remember wrong)

we have current option that could be problematic if overridden by user like --proxy no?

Hmm, I don't see this option in https://hurl.dev/docs/request.html#options

What's the best authoritative source I can always keep an eye on for inline options allowed in case I need to reject when upgrading Hurl versions?

edit: Is this the best list? https://github.com/Orange-OpenSource/hurl/blob/master/packages/hurl_core/src/parser/option.rs#L42

@jcamiel
Copy link
Collaborator Author

jcamiel commented Aug 21, 2024

Yes, you can find it also in the grammar but we've hand coded the parser from the grammar so the grammar may not be "complete" (see https://github.com/Orange-OpenSource/hurl/blob/master/docs/spec/grammar/hurl.grammar). The parser code is the best place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants