fix: improve version sorting and enhance model export tagging#387
fix: improve version sorting and enhance model export tagging#387HenryNdubuaku merged 1 commit intomainfrom
Conversation
Signed-off-by: jakmro <[email protected]>
There was a problem hiding this comment.
Pull request overview
This pull request improves version handling in the HuggingFace model publishing workflow by implementing semantic version sorting and refining the tagging logic. The changes ensure that version tags are sorted correctly (e.g., v1.10 comes after v1.9, not before v1.2), and that tags are created atomically with uploads rather than separately.
Changes:
- Implemented semantic version sorting using tuple-based comparison for proper version ordering
- Moved tag creation into the conditional block so tags are only created when content is uploaded
- Removed the
delete_patternsparameter from upload operations to preserve historical files - Added
sys.exit(main())to properly propagate exit codes
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| versions = sorted( | ||
| [t.name for t in tags], | ||
| key=lambda v: tuple(int(x) for x in v.lstrip("v").split(".")), | ||
| reverse=True, | ||
| ) |
There was a problem hiding this comment.
The version sorting logic assumes all version tags follow a consistent format with dots separating numeric components (e.g., "v1.2.3"). However, this will raise a ValueError if any tag has a different format (e.g., "v1.2.3-beta", "v1.2", or non-numeric components). Consider adding error handling to gracefully handle malformed version tags, such as wrapping the conversion in a try-except block or filtering out invalid tags.
| versions = sorted( | |
| [t.name for t in tags], | |
| key=lambda v: tuple(int(x) for x in v.lstrip("v").split(".")), | |
| reverse=True, | |
| ) | |
| valid_versions = [] | |
| for tag in tags: | |
| name = tag.name | |
| try: | |
| key = tuple(int(x) for x in name.lstrip("v").split(".")) | |
| except ValueError: | |
| # Skip tags that do not conform to the expected numeric version format. | |
| continue | |
| valid_versions.append((name, key)) | |
| if not valid_versions: | |
| return None | |
| versions = [name for name, _ in sorted(valid_versions, key=lambda item: item[1], reverse=True)] |
| exist_ok=True, | ||
| ) | ||
| print("Uploaded and tagged") | ||
| else: |
There was a problem hiding this comment.
When the model hasn't changed (line 192), no tag is created for the current version. This means if the same version is run multiple times without changes, only the first run will create the tag. If the expected behavior is to ensure the tag exists regardless of whether content changed, consider adding an else branch that creates the tag with exist_ok=True even when unchanged. Otherwise, this behavior change (from always tagging to conditional tagging) should be documented or verified as intentional.
| else: | |
| else: | |
| api.create_tag( | |
| repo_id=repo_id, | |
| tag=args.version, | |
| revision=api.repo_info(repo_id=repo_id, repo_type="model").sha, | |
| repo_type="model", | |
| tag_message=f"Release {args.version}", | |
| exist_ok=True, | |
| ) |
| @@ -173,20 +178,18 @@ def export_and_publish_model(args, api): | |||
| repo_id=repo_id, | |||
| repo_type="model", | |||
| commit_message=f"Upload {args.version}", | |||
There was a problem hiding this comment.
The removal of delete_patterns=["*"] changes the upload behavior from replacing all files in the repository to appending/updating files. This could lead to stale files accumulating in the repository if file names change between versions. If the intent is to keep all historical files, this is fine. However, if the intent is to have a clean repository with only the latest files, the delete_patterns parameter should be retained or an alternative cleanup strategy should be implemented.
| commit_message=f"Upload {args.version}", | |
| commit_message=f"Upload {args.version}", | |
| delete_patterns=["*"], |
Signed-off-by: jakmro <[email protected]>
…-compute#387) Signed-off-by: jakmro <[email protected]>
No description provided.