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

Loosen dependency versions, add resiliency #30

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Loosen dependency versions, add resiliency #30

wants to merge 12 commits into from

Conversation

WoosterInitiative
Copy link

All dependencies were pinned to specific versions which would fight with whatever project it was implemented in. Loosened them while trying to keep them reasonable.

Added a timeout for waiting for postgres to be ready.

emdoyle
emdoyle approved these changes Jun 22, 2024
View reviewed changes
Copy link
Member

@emdoyle emdoyle left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! Once the 3.9 compatibility change is made this should be good to merge.

bridge/service/postgres.py Show resolved Hide resolved
bridge/service/postgres.py Outdated Show resolved Hide resolved
build(postgres): add __future__ annotations
142ae39 
supports python 3.9

use log_info instead of print and updates a few pre-commit dependencies
@WoosterInitiative
Copy link
Author

OK, changes have been made.

I haven't been able to run tach on it, however. Seems to be an issue with pydantic? I don't have time right now to dig into that, but I also couldn't run tests on the tach package. I'm in Windows, fwiw.

@WoosterInitiative
Copy link
Author

This would also close #29

@emdoyle
Copy link
Member

emdoyle commented Jul 4, 2024

OK, changes have been made.

I haven't been able to run tach on it, however. Seems to be an issue with pydantic? I don't have time right now to dig into that, but I also couldn't run tests on the tach package. I'm in Windows, fwiw.

Apologies for the delay on this, but thanks! I think the service/docker.py file also needs the future statement but should be good to go otherwise.

The issue with Tach sounds like a known issue with backward compatibility in Tach. I'd check what version you are running and make sure it matches 0.1.2 (in dev-dependencies). If it does match, then I think ignore any failures for now since it will be a separate task to upgrade to the latest version of Tach anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emdoyle emdoyle emdoyle 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
@WoosterInitiative @emdoyle