Skip to content

Conversation

@atrocities
Copy link

Tags were previously not exposed at the endpoint level. The only tags that were effective were 'server-request-context' and 'server-limit-request-size', both of which were interpreted by conjure-rust at codegen time.

Add a 'tags' function to Endpoint, which exposes the set of tags. This is similar to the java undertow generator.

Forgoes adding 'tags' to the client, unlike the java dialogue implementation. This is because the clients are used directly in rust, as opposed to being passed into a dialogue client generator.

Tags were previously not exposed at the endpoint level. The only tags
that were effective were 'server-request-context' and
'server-limit-request-size', both of which were interpreted by
conjure-rust at codegen time.

Add a 'tags' function to Endpoint, which exposes the set of tags. This
is similar to the java undertow generator.

Forgoes adding 'tags' to the client, unlike the java dialogue
implementation. This is because the clients are used directly in rust,
as opposed to being passed into a dialogue client generator.
@changelog-app
Copy link

changelog-app bot commented Jul 28, 2025

Generate changelog in changelog/@unreleased

Type (Select exactly one)

  • Feature (Adding new functionality)
  • Improvement (Improving existing functionality)
  • Fix (Fixing an issue with existing functionality)
  • Break (Creating a new major version by breaking public APIs)
  • Deprecation (Removing functionality in a non-breaking way)
  • Migration (Automatically moving data/functionality to a new system)

Description

Expose conjure tags as endpoint parameters

Check the box to generate changelog(s)

  • Generate changelog entry

@atrocities atrocities requested a review from Copilot July 28, 2025 21:41
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for exposing Conjure tags as endpoint parameters in the Rust implementation. Previously, tags were only interpreted at codegen time for specific purposes like 'server-request-context' and 'server-limit-request-size'.

  • Adds a tags() method to the EndpointMetadata trait to expose endpoint tags
  • Implements tag parsing in the macro system to accept tags as endpoint parameters
  • Updates code generation to include tags in endpoint attribute syntax

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
conjure-macros/src/endpoints.rs Adds tag parsing logic and generates tag metadata for endpoints
conjure-http/src/server/mod.rs Extends EndpointMetadata trait with tags() method and implements for wrapper types
conjure-codegen/src/servers.rs Updates code generation to include tags in endpoint attribute syntax
Comments suppressed due to low confidence (1)

conjure-codegen/src/servers.rs:174

  • The variable name 'tags' shadows the function name 'tags' in the same scope, which reduces code clarity. Consider renaming the variable to 'tags_token' or 'formatted_tags'.
            let tags = crate::servers::tags(endpoint.tags());

Comment on lines +791 to +793
let punctuated: Punctuated<LitStr, Token![,]> = Punctuated::parse_terminated(&content)?;

for lit_str in punctuated {
Copy link

Copilot AI Jul 28, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider using a more descriptive variable name than 'punctuated' to improve code readability, such as 'tag_literals' or 'comma_separated_tags'.

Suggested change
let punctuated: Punctuated<LitStr, Token![,]> = Punctuated::parse_terminated(&content)?;
for lit_str in punctuated {
let tag_literals: Punctuated<LitStr, Token![,]> = Punctuated::parse_terminated(&content)?;
for lit_str in tag_literals {

Copilot uses AI. Check for mistakes.
@atrocities atrocities marked this pull request as ready for review July 28, 2025 21:43
@sfackler
Copy link
Member

Out of curiosity, what's the use case for doing this? My general preference up to now was to have the code generator parse out the tags and handle all of the special casing up front rather than deferring it to runtime.

@atrocities
Copy link
Author

@sfackler Use case here is to expose the 'no-response-compression' tag to the witchcraft layer. I could also just add a specific case handler to the code generator for witchcraft to pick up instead of exposing tags in general, since it's evident that there haven't really been that many tags.

@sfackler
Copy link
Member

Yeah I think the easiest thing for that is to just have conjure-codegen inject a Content-Encoding: identity header. Avoids needing to mess with both repos as well.

@stale
Copy link

stale bot commented Oct 18, 2025

This PR has been automatically marked as stale because it has not been touched in the last 14 days. If you'd like to keep it open, please leave a comment or add the 'long-lived' label, otherwise it'll be closed in 7 days.

@stale stale bot added the stale label Oct 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants