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

remove leading slash from .pyup.yml to fix bug on gitlab api #375

Merged
merged 3 commits into from
Mar 28, 2020
Merged

remove leading slash from .pyup.yml to fix bug on gitlab api #375

merged 3 commits into from
Mar 28, 2020

Conversation

ckleemann
Copy link
Contributor

As reported in #374 the support for gitlab is broken. This is due to a change in the gitlab api. Gitlab dose not accept a urlencoded leading slash in a file path. Non urlencoded leading slashes are fine. A leading slash is used to receive the pyup.yml. The the path is then urlencoded by the gitlab library.

There is no need to for the leading slash. At this point there is no previous file request which could fail a relative file path request. So I removed the slash. Which let the api call on gitlab work again.

Fix #374

@codecov
Copy link

codecov bot commented Mar 27, 2020

Codecov Report

Merging #375 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #375      +/-   ##
==========================================
+ Coverage   94.19%   94.22%   +0.02%     
==========================================
  Files          11       11              
  Lines        1155     1161       +6     
==========================================
+ Hits         1088     1094       +6     
  Misses         67       67              
Impacted Files Coverage Δ
pyup/bot.py 97.88% <100.00%> (ø)
pyup/providers/gitlab.py 85.24% <100.00%> (+0.24%) ⬆️
pyup/providers/github.py 99.39% <0.00%> (+<0.01%) ⬆️
pyup/requirements.py 88.46% <0.00%> (+0.06%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8991b40...6e6b0b7. Read the comment docs.

@rafaelpivato rafaelpivato added the considering Under consideration label Mar 28, 2020
@rafaelpivato rafaelpivato self-assigned this Mar 28, 2020
@rafaelpivato rafaelpivato added bug and removed considering Under consideration labels Mar 28, 2020
Copy link
Contributor

@rafaelpivato rafaelpivato left a comment

Choose a reason for hiding this comment

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

What did I realized is that at some point we removed adding a slash to the root here for GitHub provider: 6ae5146#diff-d486dceb95e3519b6f8ec88631bd64cc

Before that we were concerned about quoting the root path like shown here: 2fb7c01

Anyway, long-story short. Looks like you are right, neither GitHub nor GitLab needs it. I would just urge you to add another code at gitlab.Provider.get_file to lstrip any slash from path being passed. That way we help making sure that will not happen from other call.

Thanks for this, @ckleemann 🎉

@ckleemann
Copy link
Contributor Author

I removed the leading slash for pyup.yml on all places in bot.py. I also added the lstrip on the gitlab provider as suggested.
@rafaelpivato Could you merge the pr and release a new version with the fix?

Copy link
Contributor

@rafaelpivato rafaelpivato left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks @ckleemann

@rafaelpivato rafaelpivato merged commit 6d4b08a into pyupio:master Mar 28, 2020
@rafaelpivato
Copy link
Contributor

@ckleemann we are just waiting for #348 to make sure we are not breaking some stuff before publishing to PyPi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gitlab support seems to be broken
2 participants