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

Set agent strings properly #37

Closed
Tectu opened this issue Jul 2, 2021 · 4 comments · Fixed by #63
Closed

Set agent strings properly #37

Tectu opened this issue Jul 2, 2021 · 4 comments · Fixed by #63
Labels
enhancement New feature or request
Milestone

Comments

@Tectu
Copy link
Owner

Tectu commented Jul 2, 2021

Currently, agent strings are all over the place. This needs some cleaning up / more consistency.

We want to have something like malloy-client <version> and malloy-server <version>.

@Tectu Tectu added the enhancement New feature or request label Jul 2, 2021
@Tectu Tectu added this to the v0.1 milestone Jul 2, 2021
@0x00002a
Copy link
Contributor

0x00002a commented Jul 2, 2021

Could add it to the config along with #35, have a default and then let the user customise it if they really want

@Tectu Tectu self-assigned this Jul 2, 2021
@Tectu
Copy link
Owner Author

Tectu commented Jul 7, 2021

I've been wondering whether there is a scenario where a user might want to have different agent strings for different connections of the same type.
For example, given an HTTP server, would a connection ever need to have a different agent string than other HTTP connections served by the same server?

I couldn't think of any such case.

@Tectu
Copy link
Owner Author

Tectu commented Jul 10, 2021

@0x00002a how busy are you currently? I'd really like to move to the first release ASAP to make maintaining applications consuming this library easier and hopefully also attracting more users & contributors.

Do you feel like giving this a go?

@Tectu Tectu removed their assignment Jul 10, 2021
@0x00002a
Copy link
Contributor

Sure, I'll have a crack at it along with #35 tomorrow

@Tectu Tectu closed this as completed in #63 Jul 12, 2021
Tectu pushed a commit that referenced this issue Jul 12, 2021
Fixes #37

* chore: add customisable agent string to connection

* !chore: remove top level m_cfg from controller

This makes malloy::controller an abstract type... If someone needed that I would be concerned

* chore: remove top level controller init

Now called by the derived classes and `protected`

* chore: thread agent string through call stack

* chore: move top level start to protected in controller

* fix: tests build

* fix: missing return causing segfault

* feat: set Server field in header

* feat: set User-Agent field in header

* fix: server overwriting Server field in responses

* chore: add tests for agent strings

* chore: rename ws_agent_string

* chore: remove BOOST_BEAST_VERSION_STRING from agent strings

* fix: tls build

* chore: fixup docs and includes

* chore: add missing const
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants