Skip to content

Header augmentation #205

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sreya-guha
Copy link

This PR implements a configurable header system that allows administrators to include custom headers in all API responses. Headers are sent before any other content, providing important information about licensing, data retention policies, and UI components to API consumers.

Changes Made
Configuration System:

  • Added headers field to NLWebConfig dataclass in config.py
  • Enhanced load_nlweb_config() method to load headers from YAML configuration
  • Added get_headers() method to access configured headers

Response Handler:

  • Modified send_message() method in baseHandler.py to inject headers before all content
  • Added headersSent flag to ensure headers are sent only once per session
  • Supports both streaming and non-streaming response modes
  • Headers are sent immediately after API version but before "asking_sites" messages

Configuration File:

  • Updated config_nlweb.yaml with sample headers section including:
    • License information (MIT License is given as an example)
    • Specifies how long the data can be retained by the API caller
    • UI components that may be used to display the data
      More headers may be added for a specific instance of NLWeb.

Message Flow

  1. API version message (existing)
  2. Headers (new - sent as individual messages with message_type matching header key)
  3. "asking_sites" message (existing)
  4. Results and other content (existing)

Tests that were performed with the installation on hello-world

  • Test non-streaming mode: curl "http://localhost:8000/ask?query={test_query}&streaming=false"
  • Verify headers appear at top level of JSON response before other content
  • Confirm headers are sent only once per query session
    Test query that was used: "Give me episodes about foundation model differences"

@jennifermarsman jennifermarsman requested a review from Copilot June 16, 2025 22:03
Copy link
Contributor

@Copilot 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 implements a configurable header system to include custom headers in all API responses. Key changes include:

  • Integration of header sending logic in both streaming and non-streaming response modes in baseHandler.py.
  • Updates to config_nlweb.yaml to include a new headers section.
  • Modifications in config.py to load and retrieve header configurations.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
code/core/baseHandler.py Added header injection logic and a headersSent flag to guard against duplicate header sends.
code/config/config_nlweb.yaml Added a headers section with sample header entries.
code/config/config.py Extended NLWebConfig to load header configurations and provide a get_headers method.


# Send headers before any other content (but after version)
if not self.headersSent:
self.headersSent = True
Copy link
Preview

Copilot AI Jun 16, 2025

Choose a reason for hiding this comment

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

Consider setting the headersSent flag only after successfully sending all header messages. As it stands, if an exception occurs during header transmission, headersSent remains true, preventing a retry.

Suggested change
self.headersSent = True

Copilot uses AI. Check for mistakes.

license: "This data is provided under MIT License. See https://opensource.org/license/mit for details."
data_retention: "Data provided may be retained for up to 1 day."
ui_component: "This field may be used to provide a link to the web components that can be used to display the results.
Copy link
Preview

Copilot AI Jun 16, 2025

Choose a reason for hiding this comment

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

The string value for the ui_component header is missing a closing quote, which may lead to YAML parsing errors. Please ensure the string is properly enclosed in quotes.

Suggested change
ui_component: "This field may be used to provide a link to the web components that can be used to display the results.
ui_component: "This field may be used to provide a link to the web components that can be used to display the results."

Copilot uses AI. Check for mistakes.

@sreya-guha
Copy link
Author

sreya-guha commented Jun 16, 2025 via email

@jennifermarsman jennifermarsman self-assigned this Jun 18, 2025
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.

3 participants