Skip to content

Fix bug in metaflow version when public=True #2539

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
Merged
merged 2 commits into from
Aug 11, 2025
Merged

Fix bug in metaflow version when public=True #2539

merged 2 commits into from
Aug 11, 2025

Conversation

talsperre
Copy link
Collaborator

metaflow_version.get_version(public=True) doesn't return the public version if executed within a metaflow step. This is because, we return the version in the INFO file directly without any parsing. This PR addresses that.

Flow to reproduce the issue:

from metaflow import FlowSpec, step, conda

class HelloFlow(FlowSpec):
    @conda()
    @step
    def start(self):
        from metaflow import metaflow_version
        version = metaflow_version.get_version(public=True)
        print(f"Metaflow version: {version}")
        self.next(self.end)

    @step
    def end(self):
        print("HelloFlow is all done.")


if __name__ == "__main__":
    HelloFlow()

Copilot
Copilot AI reviewed Aug 11, 2025
View reviewed changes
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug where metaflow_version.get_version(public=True) returns a non-public version when executed within a Metaflow step. The issue occurs because the version from the INFO file is returned directly without parsing to remove local identifiers.

  • Introduces a make_public_version() function to convert complex version strings to PEP 440-compliant public versions
  • Updates get_version() to parse INFO file versions when public=True is requested

metaflow/metaflow_version.py
"""
Takes a complex version string and returns a public, PEP 440-compliant version.
It removes local version identifiers (+...) and development markers (-...).
"""
Copy link
Preview

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

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

The function doesn't handle the case where version_string is None or empty. This could cause an AttributeError if read_info_version() returns None and this function is called with that value.

Suggested change
"""
"""
if not version_string:
return None

Copilot uses AI. Check for mistakes.

metaflow/metaflow_version.py
# However, if we are asked for a public version, we parse it to make sure
# that no local information is included.
if public:
version = make_public_version(version)
Copy link
Preview

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

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

This line could fail if version is None, which is possible since read_info_version() can return None. The make_public_version() function should be called only after ensuring version is not None.

Copilot uses AI. Check for mistakes.

@savingoyal savingoyal force-pushed the dev/fix-public-version branch from ea8795b to 315c581 Compare August 11, 2025 19:31
@talsperre talsperre force-pushed the dev/fix-public-version branch from 5f37c40 to 3f086f5 Compare August 11, 2025 20:23
@savingoyal savingoyal merged commit e72508a into master Aug 11, 2025
28 checks passed
@savingoyal savingoyal deleted the dev/fix-public-version branch August 11, 2025 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@savingoyal savingoyal Awaiting requested review from savingoyal

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
@talsperre @savingoyal