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

[BUG] Cannot download plugins from branches that contain slashes '/' #4610

Closed
1 of 3 tasks
mbaut opened this issue Aug 2, 2024 · 2 comments · Fixed by #4614
Closed
1 of 3 tasks

[BUG] Cannot download plugins from branches that contain slashes '/' #4610

mbaut opened this issue Aug 2, 2024 · 2 comments · Fixed by #4614
Labels
bug Bug fixes

Comments

@mbaut
Copy link

mbaut commented Aug 2, 2024

Describe the problem

Parser for downloading plugins from github repo is not working correctly.

Code to reproduce issue

Lets say you have company A that has a github org, with a repo named B, and on that a branch called prod the link would be

github.com/A/B/prod

but if we have a look at the github parser in the following line, we can see the regex is ignoring "prod":

m = re.search(

System information

not needed

Other info/logs

If other people have the same problem a way to fix it is just having another word at the end
github.com/A/B/prod/this-is-ignored

the sad part is if you have a branch that is not only a word, but has a slash like "bugfix/branch" This way you will not be able to download your plugin sadly.

Willingness to contribute

The FiftyOne Community encourages bug fix contributions. Would you or another
member of your organization be willing to contribute a fix for this bug to the
FiftyOne codebase?

  • Yes. I can contribute a fix for this bug independently
  • Yes. I would be willing to contribute a fix for this bug with guidance
    from the FiftyOne community
  • No. I cannot contribute a bug fix at this time
@mbaut mbaut added the bug Bug fixes label Aug 2, 2024
@brimoor brimoor changed the title [BUG] Parser for downloading Plugins from github is bugged. [BUG] Cannot download plugins from branches that contain slashes '/' Aug 4, 2024
@brimoor
Copy link
Contributor

brimoor commented Aug 4, 2024

Hi @mbaut 👋

As documented by download_plugin(), when downloading from a specific branch, the syntax is https://github.com/<user>/<repo>/tree/<branch>, not https://github.com/<user>/<repo>/<branch>.

That said, you are 100% correct that there is a bug with the current implementation if you try to download a plugin from a branch that contains a slash. I've fixed this bug in #4614 and it will be released as of fiftyone==0.25, scheduled for release on August 13 🙌

@mbaut
Copy link
Author

mbaut commented Aug 10, 2024

Hi @brimoor,
Thank you very much for the fast reply!! Great that there already is a fix!!
Just wanted to also comment, as regex is always a bit hard to get, how it works with the fix, if I understand correctly is declared with following examples:

github.com/a/b/c
Would take User a repo b and branch c

github.com/a/b/c/d
Would take user a repo b and branch d

If you add anything after d the branch would be d/e/f...

While this works as described in the Readme provided (you should add /tree/) the fact that you should give /tree/ is not enforced and people could end up into giving a branch like github.com/user/repo/this/is/a/branch and ending up downloading branch /is/a/branch.

(I think all this is not really too important just an interesting quirk if somebody had the same problems that I did :), thanks for your effort @brimoor )

@brimoor brimoor closed this as completed Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug fixes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants