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

Modify getting Metaflow version #2009

Merged
merged 7 commits into from
Sep 16, 2024
Merged

Modify getting Metaflow version #2009

merged 7 commits into from
Sep 16, 2024

Conversation

romain-intel
Copy link
Contributor

Favors reading the INFO file if present to be able to have the most accurate version of Metaflow when executing remotely (especially in the presence of extensions).

Also limit reading the INFO file to once per process (as opposed to possibly twice).

Finally, gets the version of the source of Metaflow (and not the current directory)

Favors reading the INFO file if present to be able to have the
most accurate version of Metaflow when executing remotely (especially in
the presence of extensions).

Also limit reading the INFO file to once per process (as opposed to possibly
twice).

Finally, gets the version of the source of Metaflow (and not the current
directory)
Copy link
Collaborator

@savingoyal savingoyal left a comment

Choose a reason for hiding this comment

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

seems like this change doesn't quite work for our current set of extensions. @madhur-ob can you help triage what might be going wrong?

metaflow/metaflow_version.py Outdated Show resolved Hide resolved
metaflow/metaflow_version.py Outdated Show resolved Hide resolved
ext_version = _format_git_describe(
_call_git_describe(cwd=os.path.dirname(extension_module.__file__))
)
if ext_version is None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't this be the other way around?

also, i am getting Metaflow 2.12.18.post5-git2201f4e-dirty+ob(4.3.19) executing... - the interesting thing is that we don't have version 4.3.19 for any extensions internally. we pin the extension version explicitly to v1. it's likely that git describe --tags is the offender here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@madhur-ob do you happen to know how ob releases are git tagged?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the order is correct here:

  • look for GIT version first (ie: if we are in a GIT repo)
  • if not, look at the version field

I am not sure how you are getting the 4.3.19 indeed. This should be coming from the git_describe call that was previously used (and I did not change the logic of this) so I do suspect you are not tagging in a "normal" way.

metaflow/metaflow_version.py Outdated Show resolved Hide resolved
metaflow/metaflow_version.py Outdated Show resolved Hide resolved
romain-intel and others added 5 commits September 11, 2024 09:19
* Add installed_extensions information

This adds information to the system tags about the metaflow extensions that
are installed. This makes it easier to reproduce the exact same
environment. Information about whether the package information is complete or
not is also included.

This PR also fixes some issues with extension loading and allows extension
information to be gathered in a programatic fashion thereby enabling the
discovery of extensions.

* Allow users to configure where extensions are searched

* Move EXTENSIONS_SEARCH_DIRS out of config

* Move the extension installation information to _graph_info
@romain-intel romain-intel dismissed savingoyal’s stale review September 16, 2024 06:24

Savin confirmed this looked good offline.

@romain-intel romain-intel merged commit a85ee20 into master Sep 16, 2024
26 checks passed
@romain-intel romain-intel deleted the fix/version-info branch September 16, 2024 18:28
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.

3 participants