Skip to content

Conversation

@rajkumardongre
Copy link
Contributor

This procedure adds the header only if it does not already exist in the current header array. It is typically used internally to add default headers.

@milancurcic
Copy link
Member

Thanks @rajkumardongre. The problem here is that we're now duplicating code between this and the http_response module where you also have append_header. I suggest that you define all header related functions in the http_header module, and reuse them from there in request, response, and client modules. The header procedures won't be type bound because they work on arrays of header_type, but they would be defined in the appropriate scope and without duplicating code.

@rajkumardongre
Copy link
Contributor Author

Thanks @milancurcic for the review. I have migrated all header-related procedures (get_header_value, put_if_header_absent, append_header) to the src/http/http_header.f90 file.

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. I have simplified the headers API. Rather than introducing a new procedure that does two things (check for presence of key and call append_header), I have removed put_if_header_absent and introduced header_has_key function which simply checks for existence of a key. This makes the API more horizontal in the sense that independent functionality is orthogonal and each procedure does only one thing.

I have also introduced a test suite for headers. Going forward, for any new functionality and module, please also add a test suite program. I have also added and edited existing docstrings in http_header.f90 into FORD-style docstrings. This will allow us easy generation of browsable API docs. Going forward, please use the same dosctring style when documenting functions.

I'm still not 100% happy with how we're setting default headers in the new_request function. Currently we're duplicating the default header keys and values. I'll think about how better to approach this but for now it's fine.

@milancurcic milancurcic merged commit 691ad82 into fortran-lang:main Jun 19, 2023
@rajkumardongre rajkumardongre deleted the set_default_header branch July 2, 2023 06:33
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.

4 participants