Skip to content

Add describe, delete, and list namespaces (REST) #507

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

Merged
merged 17 commits into from
Jun 17, 2025
Merged

Conversation

rohanshah18
Copy link
Contributor

@rohanshah18 rohanshah18 commented May 30, 2025

Problem

Add support for describe, delete, and and list namespaces for index and async index classes.

Solution

Following methods were added to the index and async index classes:

  1. describe_namespace(self, namespace: str, **kwargs) -> "NamespaceDescription"
  2. delete_namespace(self, namespace: str, **kwargs) -> Dict[str, Any]
  3. list_namespaces:
    - list_namespaces(self, **kwargs) -> Iterator[ListNamespacesResponse] (for index.py)
    - list_namespaces(self, **kwargs) -> AsyncIterator[ListNamespacesResponse] (for index_asyncio.py)
  4. list_namespaces_paginated(
    self, limit: Optional[int] = None, pagination_token: Optional[str] = None, **kwargs
    ) -> ListNamespacesResponse

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Infrastructure change (CI configs, etc)
  • Non-code change (docs, etc)
  • None of the above: (explain here)

Test Plan

Added integration tests for both index and async index classes

@rohanshah18 rohanshah18 marked this pull request as ready for review June 6, 2025 18:44
@rohanshah18 rohanshah18 changed the title Add describe, delete, and list namespaces Add describe, delete, and list namespaces (REST) Jun 9, 2025
Copy link
Contributor

@austin-denoble austin-denoble left a comment

Choose a reason for hiding this comment

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

LGTM overall, left some comments and a few questions. Nice work, I'll defer to @jhamon for now on a lot of this as I'm still getting familiar with the client refactor.

Copy link
Collaborator

@jhamon jhamon left a comment

Choose a reason for hiding this comment

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

A couple things:

Require kwargs for new methods

I would add the @require_kwargs decorator on all new methods. Search the repo for other examples of this. We haven't added it in 100% of places because it would be a breaking change to add it to methods that never had it before if people are successfully calling methods using positional arguments, but we want to use this on all new methods going forward.

Why? Well, python is flexible in how methods are called. If you had a function like this:

def mymethod(foo):
  print(foo)

then you could call the function with positional arguments like mymethod('bar') or kwarg-style as mymethod(foo='bar'). Relying on positional arguments can cause problems down the road when new arguments are added, the args list gets long, if arguments transition from required to optional, etc. If everyone uses the keyword form from the beginning, a lot of these changes can be made without disrupting existing code.

Testing docs change

Unfortunately testing these docs changes is slightly involved because there's a problem preventing us from directly having sphinx as a dev-dependency. If you want to try building the docs, you need to follow the steps in this github action but do not commit any changes to the python version or poetry.lock when you are done.

Basically, you need to

  • poetry env use 3.11 to make sure you're using python 3.11
  • (Temporarily, do not commit) edit the pyproject.toml to require python ^3.11
  • poetry install -E asyncio -E grpc
  • Install docs deps poetry add sphinx myst-parser --group dev
  • Build the docs poetry run sphinx-build -b html docs docsbuild
  • Inspect the output open docsbuild/index.html

You will need to edit docs/rest.rst and docs/asyncio.rst to explicitly expose docs for your new methods, and then you'll want to look at them to see whether the generated text looks correct from a formatting standpoint.

Copy link
Collaborator

@jhamon jhamon left a comment

Choose a reason for hiding this comment

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

I left a ton of comments (sorry lol), but overall I think you did a nice job.

@rohanshah18 rohanshah18 merged commit 8553b93 into main Jun 17, 2025
35 checks passed
@rohanshah18 rohanshah18 deleted the rshah/namespaces branch June 17, 2025 15:12
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