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

fix: do not use test util in package #464

Merged
merged 5 commits into from
Sep 24, 2024
Merged

fix: do not use test util in package #464

merged 5 commits into from
Sep 24, 2024

Conversation

anitarua
Copy link
Contributor

@anitarua anitarua commented Sep 23, 2024

Ran into an error while writing new dev docs examples. Created a new str_to_bytes util function in the package proper rather than using the one from tests.utils. Bug appears to be fixed when running the examples against a locally built version of the package now.

File "/Users/anita/client-sdk-python/examples/.venv/lib/python3.10/site-packages/momento/internal/_utilities/_permissions.py", line 28, in <module>
    from tests.utils import str_to_bytes
ModuleNotFoundError: No module named 'tests'

Also moved the two validators to internal utils where the others are.

@anitarua anitarua requested review from malandis and a team September 23, 2024 23:20
@anitarua anitarua marked this pull request as ready for review September 23, 2024 23:20
rishtigupta
rishtigupta previously approved these changes Sep 23, 2024
Copy link
Contributor

@rishtigupta rishtigupta left a comment

Choose a reason for hiding this comment

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

:shipit:

@malandis
Copy link
Contributor

While we're here, the validators are best placed alongside the ones here

@malandis
Copy link
Contributor

Lastly could you roll up the names from the utilities to the utilities__init__? Should make the imports easier throughout the project.

Copy link
Contributor

@malandis malandis left a comment

Choose a reason for hiding this comment

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

Also need the init file for utilities that rolls up names from the various modules therein.

src/momento/cache_client.py Outdated Show resolved Hide resolved
@anitarua anitarua requested a review from malandis September 24, 2024 18:37
@anitarua anitarua merged commit a558c0b into main Sep 24, 2024
10 checks passed
@anitarua anitarua deleted the bug-fix branch September 24, 2024 18:47
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