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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump pylint, pydantic, remove "artifacts.yaml" logic #401

Closed
wants to merge 11 commits into from

Conversation

aguschin
Copy link
Contributor

@aguschin aguschin commented Sep 5, 2023

馃憖

This will require Studio BE update - we need to remove old logic that works with artifacts.yaml - that will make Studio MR backend codebase more easy to work with! To clarify, before we already removed "write" access to old artifacts.yaml, but kept "read" access. Now we remove "read" access as well. (Please notice, that will lead to some users losing info about model versions that are 3+ months old, but I hope that's ok).

So once this merged, consider bumping major or minor version, but not the patch.

@codecov-commenter
Copy link

codecov-commenter commented Sep 5, 2023

Codecov Report

Patch coverage is 96.87% of modified lines.

Files Changed Coverage
gto/api.py
gto/cli.py 0.00%
gto/utils.py
gto/config.py 96.66%
gto/__init__.py 100.00%
gto/base.py 100.00%
gto/git_utils.py 100.00%
gto/registry.py 100.00%
gto/tag.py 100.00%

馃摙 Thoughts on this report? Let us know!.

@aguschin
Copy link
Contributor Author

aguschin commented Sep 5, 2023

cc @amritghimire about changes required in BE ^

@aguschin
Copy link
Contributor Author

aguschin commented Sep 5, 2023

@pmrowla, I assume this is what you meant to make it compatible with py-template. There are few things still that should be fixed - like remove top-level silence statements here https://github.com/iterative/gto/blob/bumping-versions/gto/config.py#L1, but overall it should be ready for py-template I hope!

@aguschin aguschin self-assigned this Sep 5, 2023
@aguschin
Copy link
Contributor Author

aguschin commented Sep 8, 2023

closes #400

@shcheklein
Copy link
Member

Since studio depends for now on MLEM and MLEM depends on the first Pydantic, will those co-exist fine together or does it means we need to upgrade / drop MLEM dependency?

@skshetry skshetry temporarily deployed to pypi September 14, 2023 08:31 — with GitHub Actions Inactive
@skshetry skshetry temporarily deployed to pypi September 14, 2023 08:47 — with GitHub Actions Inactive
skshetry added a commit that referenced this pull request Sep 14, 2023
Split out from #401 to make it easier to review. Some test changes and behaviour changes have been dropped.
skshetry added a commit that referenced this pull request Sep 14, 2023
Split out from #401 to make it easier to review. Only cosmetic changes
have been made and some linters errors are suppressed.
@skshetry skshetry temporarily deployed to pypi September 14, 2023 12:07 — with GitHub Actions Inactive
@skshetry skshetry temporarily deployed to pypi September 14, 2023 12:48 — with GitHub Actions Inactive
@skshetry
Copy link
Member

skshetry commented Sep 14, 2023

We discussed this in today's team meeting and decided not to merge this PR due to the breakages involved. We don't want to risk merging this for now.

Most of the pylint/mypy changes has been cherry-picked into the main branch, and CI is now running from Python 3.8 - 3.11 versions now.

I have created #413 that supports both Pydantic v1 and v2, so there's no immediate need to migrate to use pydantic v2 APIs too.

(For future reference, afddc5e is the last commit before it was rebased).

@skshetry skshetry temporarily deployed to pypi September 15, 2023 06:08 — with GitHub Actions Inactive
@skshetry skshetry closed this Sep 15, 2023
@skshetry skshetry deleted the bumping-versions branch September 15, 2023 06:10
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.

None yet

4 participants