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

fix prototype resource loading #5447

Merged
merged 2 commits into from
Feb 21, 2022
Merged

Conversation

pmeier
Copy link
Collaborator

@pmeier pmeier commented Feb 21, 2022

The current logic checks for any files with the same stem regardless of the ending

path_candidates = {file for file in path.parent.glob(path.name.replace("".join(path.suffixes), "") + "*")}

It should be checking for the same stem and a different suffix which starts with a .. Without this patch, a file like foo_bar.zip is a valid path candidate for foo.zip.

This was independently discovered in

@facebook-github-bot
Copy link

facebook-github-bot commented Feb 21, 2022

💊 CI failures summary and remediations

As of commit 6a34451 (more details on the Dr. CI page):


💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

Stamping to unlock.

If the logic starts to become complex and with various edge-cases that we're not seeing yet (like the one that just occured), it might be worth considering a different approach for this eventually

@pmeier pmeier merged commit c530b62 into pytorch:main Feb 21, 2022
facebook-github-bot pushed a commit that referenced this pull request Feb 25, 2022
Summary:
* fix prototype resource loading

* revert unrelated change

Reviewed By: jdsgomes

Differential Revision: D34475313

fbshipit-source-id: 9064d3ce3759ffa980b460924dd5815a87d7df5f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants