Skip to content

Conversation

rajkumardongre
Copy link
Contributor

No description provided.

@rajkumardongre rajkumardongre added the enhancement New feature or request label Jul 10, 2023
Copy link
Member

@milancurcic milancurcic left a comment

Choose a reason for hiding this comment

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

Thanks @rajkumardongre, mostly looks good, I added a few requests. Also, tests for timeout are missing.

@rajkumardongre
Copy link
Contributor Author

Thanks @milancurcic, I have made the required changes

@rajkumardongre
Copy link
Contributor Author

Thanks, @interkosmos, I have made the required changes

@milancurcic
Copy link
Member

@rajkumardongre can you resolve the conflicts (just merging main into this branch may be sufficient) and merge this PR when ready?

@milancurcic
Copy link
Member

I think httpbin is flaky here (test failing sometimes for basic auth). Any other service we can use for this?

@rajkumardongre
Copy link
Contributor Author

I think httpbin is flaky here (test failing sometimes for basic auth). Any other service we can use for this?

Thanks @milancurcic, I found https://postman-echo.com/ as an alternative.

! setting username and password
auth = pair_type('user', 'passwd')
res = request(url='https://httpbin.org/basic-auth/user/passwd', auth=auth)
res = request(url='https://postman-echo.com/get', auth=auth)
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this need to be?

Suggested change
res = request(url='https://postman-echo.com/get', auth=auth)
res = request(url='https://postman-echo.com/basic-auth', auth=auth)
@rajkumardongre
Copy link
Contributor Author

I was considering whether we should make the procedures pair_has_name, get_pair_value, and append_pair from the http_pair module available to the top-level API by importing them into the http module.

@milancurcic
Copy link
Member

Is it important for this PR? It seems to me like a separate issue.

We can consider pair_has_name (sounds weird, let's re-evaluate the name) and get_pair_value if there are good use-cases. append_pair I think is a candidate for removal, since Fortran itself has a simpler syntax to do that.

@rajkumardongre
Copy link
Contributor Author

Ok, we can discuss it on other PR

@milancurcic milancurcic merged commit 20fa99e into fortran-lang:main Jul 11, 2023
@rajkumardongre rajkumardongre deleted the timeout branch July 13, 2023 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
3 participants