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

Locking keripy dependency #163

Closed
iFergal opened this issue Jan 4, 2024 · 8 comments · Fixed by #242
Closed

Locking keripy dependency #163

iFergal opened this issue Jan 4, 2024 · 8 comments · Fixed by #242
Assignees

Comments

@iFergal
Copy link
Collaborator

iFergal commented Jan 4, 2024

Opening a new issue for this more generally but #152 already mentions this. Would it be OK if we pin the githash of keripy used? e.g. keri @ git+https://git@github.com/WebOfTrust/keripy.git@2d530601d3463d820229de0676cda519523b73ca

Right now we are lacking determinism for building KERIA - building older versions typically break, and even the latest can break if it falls slightly behind keripy.

This in general causes problems with deployment to environments with multiple team members, git bisect, CICD determinism etc.

@psteniusubi
Copy link
Contributor

What's a working strategy for this? Can it be automated?

Right now integration tests in signify-ts are failing if running with the latest development branches of keria and keripy. I have to roll back both keria and keripy to make the tests running.

@iFergal
Copy link
Collaborator Author

iFergal commented Jan 5, 2024

I don't think it needs to be automated though, or if it even could - it's just a dependency version. development would point to a certain githash that works, and then if a feature branch of keria requires an update of keripy, that PR should also update the githash. (this unfortunately requires some coordination between branches - a branch that's open for longer shouldn't roll back to an older githash but I think that should be a merge conflict to resolve anyway)

Automatically updating the githash would mean automatically making the code changes in KERIA if there were breaking changes.

My current solution when tests fail is to find the githash on keripy that occured right before the latest commit to KERIA. :P

@lenkan
Copy link
Collaborator

lenkan commented Jan 8, 2024

I agree that we need a solution to ensure deterministic builds. From my point of view, just pinning the dependency to a commit seems reasonable and simple enough. The workflow to manually update the dependency does not seem to add any significant overhead.

@m00sey m00sey self-assigned this Jan 11, 2024
@iFergal
Copy link
Collaborator Author

iFergal commented Feb 1, 2024

@m00sey Bump =)

Right now in my team we are sharing a manually deployed KERIA instance (/pointing to local KERIAs in dev) so I can work around this, but we are soon changing to infra as code so deterministic builds would be awesome!

@lenkan
Copy link
Collaborator

lenkan commented Apr 19, 2024

@iFergal can this be closed now since Keria is pointing to a published pypi package of keripy?

keria/setup.py

Line 79 in 4217166

'keri>=1.2.0-dev0',

@iFergal
Copy link
Collaborator Author

iFergal commented Apr 19, 2024

@lenkan I'm not super familiar with Python deps but I think this means it would also use version 1.3.0 or even 2.0.0 if available - it seems there's no lockfile concept here. Or does the dev tag affect that?

It's probably OK since releases will be coordinated, but it would be a headache if we were trying to find the root cause of a regression using git bisect or something.

@iFergal
Copy link
Collaborator Author

iFergal commented May 14, 2024

This still applies; at time of writing setup.py relies on >=1.2.0-dev0 but in a fresh venv it is pulling 1.2.0-dev2 which is not compatible.

@pfeairheller OK if we use == instead? Or use poetry/pyproject.toml instead so we have a lockfile? Not sure on what's involved, or if we should, but happy to give a try.

@pfeairheller
Copy link
Member

Interesting that pip will roll up to a "dev2", I assumed it wouldn't do that. Switching to an "==" is the right fix here.

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.

5 participants