jupyterlab / pull-requests

A JupyterLab extension for reviewing GitHub pull requests
BSD 3-Clause "New" or "Revised" License
37 stars 18 forks source link

prevent appending pr_id urls with base_api_url #61

Closed meihkv closed 2 years ago

meihkv commented 2 years ago

Only url_path_join(self.base_api_url, url) if pr_id = get_request_attr_value(self, “id”) already contains a fully qualified url. Example: If pr_id already starts with http, then don’t prefix it with base_api_url. This occurs within Enterprise Github where we need to specify a custom base api url such as http://api.githubprod.com

welcome[bot] commented 2 years ago

Thanks for submitting your first pull request! You are awesome! :hugs:
If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly. welcome You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! :wave:
Welcome to the Jupyter community! :tada:

codecov-commenter commented 2 years ago

Codecov Report

Merging #61 (a7ad0ef) into master (fe2fdf3) will increase coverage by 0.12%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #61      +/-   ##
==========================================
+ Coverage   61.14%   61.26%   +0.12%     
==========================================
  Files          33       33              
  Lines        1647     1647              
  Branches       94       94              
==========================================
+ Hits         1007     1009       +2     
+ Misses        635      633       -2     
  Partials        5        5              
Impacted Files Coverage Δ
jupyterlab_pullrequests/managers/manager.py 91.26% <100.00%> (+1.94%) :arrow_up:

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 fe2fdf3...a7ad0ef. Read the comment docs.

fcollonval commented 2 years ago

Hey @meihkv thanks for reaching out. I'm not sure to understand your issue (and therefore this PR). If you are using a custom GitHub server, you need to set PRConfig.api_base_url to your server url - aka http://api.githubprod.com (see README).

meihkgnuov commented 2 years ago

Hello, I have it correctly set with: c.PRConfig.api_base_url = 'https://api.githubprod.prci.com/'

I can confirm that it read my custom API URL because I can retrieve a list of pull requests:

image

It gets me this far. However, when I click on a PR, I get the following error: Error: Invalid response: 404 Invalid response in 'https://api.githubprod.prci.com/https://githubprod.prci.com/api/v3/repos/...

This means that it's trying to append the base_api_url to the pr_id. The correct request should be without the base_api_url: https://githubprod.prci.com/api/v3/repos/...

self.base_api_url = https://api.githubprod.prci.com/

Hence, this pull request is ensuring that if the url is already valid, then there's no need to append the base api url with it.

fcollonval commented 2 years ago

Thanks for the clarification.

Does that mean that https://api.githubprod.prci.com/ is an alias for https://githubprod.prci.com/api?

meihkgnuov commented 2 years ago

Sorry, I don't know what an alias means in this context, so I'm not sure how to answer that. What can I do to check if it's an alias?

The extension makes the correct _call_provider() every time with https://api.githubprod.prci.com/, but for pull requests, it doesn't need self.base_api_url. I don't know if that's specific to my organization or if that behavior applies to all custom PRConfig.api_base_url when specifying a Github Enterprise url.

welcome[bot] commented 2 years ago

Congrats on your first merged pull request in this project! :tada: congrats Thank you for contributing, we are very proud of you! :heart: