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

optional "client_name" additional property in clients to specialize user-agent? #173

Closed
hemidactylus opened this issue Jan 19, 2024 · 4 comments · Fixed by #190
Closed
Assignees

Comments

@hemidactylus
Copy link
Collaborator

to be prepended to user-agent if provided (with separating slash).
Very back-compatible.

@hemidactylus
Copy link
Collaborator Author

A design decision is in order here. The context is, a future LangChain will use a future astrapy and set this optional "client name", which goes all the way to the user agent.

LangChain's classes (as many other similar plugins) accept both the credentials to create their AstraDB instance, or a ready-made instance provided from outside. What should then the plugin do? (and how should astrapy's API be like?)

  1. the plugin calls astra_db.set_client_name("langchain")?
  2. the setting is at collection level, i.e. astra_db_collection.set_client_name("langchain")?
  3. no properties, just an optional parameter to each method that results in API calls?

Now,

  • 3 is unwieldy to maintain
  • 1 is not something the plugin has the right to do since the astra_db object can in principle be shared by other code (plus, once it spawns an AstraDBCollection it does not really "talk to it" anymore).
  • 2 seems to be the right place. With some care, as the plugin code in Langchain (that we take for a blueprint of most plugins) also does occasional calls through the AstraDB class, viz. DDL.

So, even if going with 2, if we want to properly characterize DDL calls with their "client_name" we need at least

  • an additional param in one-off methods of AstraDB for DDL
  • either a param in each collection method (maintenance-heavy) or a collection-level property that gets used in each request.

Choosing param for AstraDB and instance property for AstraDBCollection is ... ugly and asymmetrical.

@hemidactylus hemidactylus self-assigned this Feb 1, 2024
@kentnagase
Copy link

Do we have enough information to make a decision on the design options shared above?

@hemidactylus
Copy link
Collaborator Author

Well, this is mostly an internal issue of maintainability/flexibility, with little exposure to code using astrapy (such as the LangChain plugin) and virtually zero to end user, so we can go with what looks like a least-effort satisfactory choice and adjust later (though hopefully not).

@kentnagase
Copy link

Thank you for this information, do we have an idea for when this can be prioritized for implementation? I'm coordinating the downstream changes needed to surface in BQ and MixPanel, and the upstream integrations (ex. Llamaindex, Langchain)

hemidactylus added a commit that referenced this issue Feb 3, 2024
* replaced the ApiRequestHandler classes with functions

* add user-agent function and get rid of all the default params in utils/api stack

* caller_name/version infrastructure ready to use by ops/db

* add headers test dependency and sample test for user-agent

---------

Co-authored-by: Eric Hare <ericrhare@gmail.com>
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 a pull request may close this issue.

2 participants