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.

Sign up for GitHub

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

Jump to bottom

feat: cursor pagination of get_all_users in /admin/users route #1441

Merged
merged 5 commits into from
Jun 10, 2024
Merged

feat: cursor pagination of get_all_users in /admin/users route #1441

merged 5 commits into from
Jun 10, 2024

Conversation

ajanitshimanga
Copy link
Contributor

@ajanitshimanga ajanitshimanga commented Jun 7, 2024 •
edited
Loading

Please describe the purpose of this pull request.
Is it to add a new feature? Is it to fix a bug?
cursor pagination for get_all_users in /admin/users route

How to test
How can we test your PR during review? What commands should we run? What outcomes should we expect?
poetry run pytest -s tests

specifically the test_admin_client tests: poetry run pytest -s tests/test_admin_client.py

Have you tested this PR?
Have you tested the latest commit on the PR? If so please provide outputs from your tests.
Yes, Tested and are passing locally

Related issues or PRs
Please link any related GitHub issues or PRs.
#1156

Is your PR over 500 lines of code?
If so, please break up your PR into multiple smaller PRs so that we can review them quickly, or provide justification for its length.

Additional context
Add any other context or screenshots about the PR here.

@ajanitshimanga ajanitshimanga mentioned this pull request Jun 7, 2024
Closed
@cpacker cpacker requested a review from sarahwooders June 8, 2024 00:12
@ajanitshimanga
Copy link
Contributor Author

ajanitshimanga commented Jun 9, 2024 •
edited
Loading

Tests are passing locally. Just pushed a new commit addressing the max connection error that was occurring in CI workflow due to max connections being reached from user cleanup fixture.

Screenshot 2024-06-09 at 4 17 02 PM

@ajanitshimanga
Copy link
Contributor Author

ajanitshimanga commented Jun 9, 2024 •
edited
Loading

Investigating singular test failure in tests/test_base_functions.py. This test is passing locally for me, looking into this.

Local passing:
Screenshot 2024-06-09 at 4 27 08 PM

Relevant info:

=================================== FAILURES ===================================
________________________________ test_archival _________________________________

agent_obj = <memgpt.agent.Agent object at 0x7f1f56ecf3e0>

    def test_archival(agent_obj):
>       base_functions.archival_memory_insert(agent_obj, "banana")

tests/test_base_functions.py:41: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
memgpt/functions/function_sets/base.py:161: in archival_memory_insert
    self.persistence_manager.archival_memory.insert(content)
memgpt/memory.py:445: in insert
    raise e
memgpt/memory.py:419: in insert
    embedding = self.embed_model.get_text_embedding(text)
memgpt/embeddings.py:135: in get_text_embedding
    return self._call_api(text)
memgpt/embeddings.py:117: in _call_api
    response_json = response.json()
.venv/lib/python3.12/site-packages/httpx/_models.py:761: in json
    return jsonlib.loads(self.content, **kwargs)
/opt/hostedtoolcache/Python/3.12.3/x64/lib/python3.12/json/__init__.py:346: in loads
    return _default_decoder.decode(s)
/opt/hostedtoolcache/Python/3.12.3/x64/lib/python3.12/json/decoder.py:337: in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <json.decoder.JSONDecoder object at 0x7f1f852e5ac0>
s = '<!DOCTYPE html>\n<!--[if lt IE 7]> <html class="no-js ie6 oldie" lang="en-US"> <![endif]-->\n<!--[if IE 7]>    <html ...istener("DOMContentLoaded",d)})();</script>\n</div><!-- /.error-footer -->\n\n\n    </div>\n</div>\n</body>\n</html>\n'
idx = 0

    def raw_decode(self, s, idx=0):
        """Decode a JSON document from ``s`` (a ``str`` beginning with
        a JSON document) and return a 2-tuple of the Python
        representation and the index in ``s`` where the document ended.
    
        This can be used to decode a JSON document from a string that may
        have extraneous data at the end.
    
        """
        try:
            obj, end = self.scan_once(s, idx)
        except StopIteration as err:
>           raise JSONDecodeError("Expecting value", s, err.value) from None
E           json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

/opt/hostedtoolcache/Python/3.12.3/x64/lib/python3.12/json/decoder.py:355: JSONDecodeError

sarahwooders
sarahwooders approved these changes Jun 10, 2024
View reviewed changes
Copy link
Collaborator

@sarahwooders sarahwooders left a comment

Choose a reason for hiding this comment

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

lgtm! Not sure why the tests are failing but probably an issue with the tests, not the PR

@sarahwooders sarahwooders merged commit 0ceea24 into letta-ai:main Jun 10, 2024
8 of 11 checks passed
@ajanitshimanga
Copy link
Contributor Author

Awesome, thanks! Looking forward to further contributions :)

mattzh72 pushed a commit that referenced this pull request Oct 9, 2024
@ajanitshimanga
feat: cursor pagination of get_all_users in /admin/users route (#1441)
846a681
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sarahwooders sarahwooders sarahwooders approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ajanitshimanga @sarahwooders