GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Pass a header into calls to mixer for usage logs by lucyking140 · Pull Request #276 · datacommonsorg/api-python

Conversation

lucyking140 commented Sep 20, 2025 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Adds a clientlib-python header to the newer datacommons_client client library
- This does not include the older datacommons client because it makes calls to the V1 API, which will be deprecated soon.
These are used in Mixer's usage logs
The header accepts an optional surface_header_value tag in the observation endpoints so that other DC surfaces that make calls to the client library (MCP server, DataGemma, etc.) can be tracked.
- Theoretically, others could also pass in other values for this tag if they make calls directly to the python client library. Like with custom DCs, I plan to accept that this might happen in rare cases because these usage logs will be aggregated over many queries.

Tests:

Checked with local mixer that the metadata is correctly updated in the clientlib-python case and when we pass in metadata_source
Modified current test suite to include this header in the expected response.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

lucyking140 added 11 commits

September 20, 2025 14:11


          add header for old and new

ea9559c


          adding a passed-in param to account for calls made from MCP, etc.

a468864


          adding a passed-in param to account for calls made from MCP, etc.

d985ef4


          adding flag to other fetch calls

de9cd61


          adding basic validation:

af0d321


          updating test suite to account for new metadata tag

1cdd6c3


          removing old client changes because it uses V1

99a1323


          cleaning up docs


          removing -new because we only have one endpoint here

0ded695


          removing -new because we only have one endpoint here


          removing -new because we only have one endpoint here

a8a22e5

lucyking140 marked this pull request as ready for review

September 22, 2025 15:30

hqpho commented Sep 22, 2025

Copy link

Copy Markdown

Collaborator

/gcbrun

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

hqpho self-requested a review

September 22, 2025 15:34

hqpho reviewed

Sep 22, 2025

View reviewed changes

hqpho left a comment

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

In principle, LG! Nitty naming thing: since both "metadata" and "source" are either already or likely to soon become overloaded terms in the DC ecosystem, WDYT of calling the parameter something like surface_header_value or feature_surface_tag or something like that?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

datacommons_client/endpoints/base.py Outdated

		# if this call originates from another DC product (MCP server, DataGemma, etc.), we indicate that to Mixer
		if(metadata_source):
		# makes it clearer to public users that this is a tag specific to other DataCommons features
		if(metadata_source != "mcp" and metadata_source != "datagemma"):

hqpho Sep 22, 2025

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Optional: Define a constant array of allowed values somewhere

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

lucyking140 Sep 22, 2025

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Added a list! Also expanded this to include "mcp-{some number}" to support MCP versions like Christie suggested. Also renamed the param to surface_header_value!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          updating run_test to temporarily change click versions

hqpho commented Sep 22, 2025

Copy link

Copy Markdown

Collaborator

/gcbrun

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

lucyking140 added 2 commits

September 22, 2025 13:44


          adding support for mcp-version

12d08e0


          adding -y to uninstall click command to make it work without input

1015c3c

hqpho commented Sep 22, 2025

Copy link

Copy Markdown

Collaborator

/gcbrun

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          correcting tests

5c05728

lucyking140 requested a review from hqpho

September 22, 2025 18:43

lucyking140 mentioned this pull request

Sep 22, 2025

Adding metadata_source tag to calls to Python client library for usage logs datacommonsorg/agent-toolkit#86

Merged

hqpho approved these changes

Sep 22, 2025

View reviewed changes

hqpho left a comment

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This change will require a PyPi release to take effect. Let's coordinate that after it's merged.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

datacommons_client/utils/error_handling.py

		Raised when a surface_header_value is passed in that is not a valid Data Commons surface.
		This value is used in the DC usage logs in Mixer
		"""
		default_message = "The surface header value should only to indicate a call made from Data Commons surfaces like the MCP server or DataGemma."

hqpho Sep 22, 2025

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Possible edit: "The surface header value must be a surface known to the Data Commons team."

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

lucyking140 Sep 23, 2025

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Done!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          converting to a param passed in on init

c072f33

hqpho approved these changes

Sep 22, 2025

View reviewed changes

Comment thread

datacommons_client/endpoints/base.py Outdated


		url = (self.base_url if endpoint is None else f"{self.base_url}/{endpoint}")
		headers = self.headers
		# if this call originates from another DC product (MCP server, DataGemma, etc.), we indicate that to Mixer

hqpho Sep 22, 2025

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Optional: mention the default here

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

lucyking140 Sep 23, 2025

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Added! And moved this to the new location where I'm setting the header

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

lucyking140 added 4 commits

September 22, 2025 17:51


          pushing to see if i have build privileges

4657b00


          cleaned up tests to match new format

fad438e


          converting to an attribute of the DCC

c19bcec


          correcting typos

dc45042

lucyking140 added 2 commits

September 23, 2025 10:07


          cleaning up docs

81b6a23


          adding tests to make sure the surface header is propogated correctly

76b573d

hqpho approved these changes

Sep 23, 2025

View reviewed changes

Comment thread

datacommons_client/endpoints/base.py Outdated

		dc_instance = "datacommons.org"

		self.headers = build_headers(api_key)
		self.api_key = api_key if api_key else None

hqpho Sep 23, 2025

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Is self.api_key referenced anywhere else? If not, consider passing it as an explicit additional param to build_headers.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

lucyking140 Sep 23, 2025

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Moved it to a param!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

datacommons_client/tests/endpoints/test_request_handling.py

		@patch("requests.post")
		def test_send_post_request_surface_header(mock_post):
		"""Tests a successful POST request with a surface header."""
		mock_response = MagicMock()

hqpho Sep 23, 2025

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Nit and usually would be auto-formatted in other repos: I think there needs to be a blank line after the comment

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

lucyking140 Sep 23, 2025

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Added!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

lucyking140 added 5 commits

September 23, 2025 14:36


          making api key an explicit param in build-headers

b3b21b2


          fixing spacing

08261ea


          linting


          undoing notebook changes:

6d12ff5


          Merge branch 'master' of https://github.com/datacommonsorg/api-python …

235ec86

…into lucysking/clientlib-metadata

lucyking140 merged commit bd578ff into datacommonsorg:master

Oct 2, 2025

Reviewers

hqpho

hqpho approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

lucyking140 commented Sep 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hqpho commented Sep 22, 2025

Uh oh!

hqpho left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hqpho commented Sep 22, 2025

Uh oh!

hqpho commented Sep 22, 2025

Uh oh!

hqpho left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lucyking140 commented Sep 20, 2025 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.