-
-
Notifications
You must be signed in to change notification settings - Fork 201
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
feat(bitbucket): support self-hosted instances #763
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #763 +/- ##
==========================================
- Coverage 40.02% 38.64% -1.37%
==========================================
Files 20 22 +2
Lines 1642 1703 +61
==========================================
+ Hits 657 658 +1
- Misses 985 1045 +60
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM overall, I'm thinking if we should merge this functionality with the bitbucket
feature and use the cloud API whenever a custom bitbucket URL is provided. I think it will make things more clean, what do you think?
Also sorry for the delay. I'm totally happy to proceed with this PR and I hope you're planning to work on it 🐻
@orhun absolutely I'll continue on it. Just haven't have much time lately to continue on where I started. I really like the idea of switching the implementation based on whether a custom URI was used. Felt really odd to have these two configs despite they pretty much achieving the same. A few things are obviously still missing here and there. Especially testing is an issue I guess. To have a self-hosted Bitbucket server running is easy as you can just grab the latest Docker image and run it, but then you need to get a 30-day license and so on. Way too many steps to verify the implementation. As far as I understand the API isn't really evolving much (maybe the server variant gets the update to API v2 at some point eventually). So my idea is to record a few queries and save them in the repo, anonymized of course. Then we can see if we use a full blown HTTP mocking crate or simply deserialize the files into expected responses. |
Thanks for showing interest! Just an idea, maybe it makes sense to start this PR over since we won't have two separate functionalities for this :)
Wow, that's really bad ;/
Loved this idea! This will also save us from spurious CI errors due to networking issues that happen occasionally with GitLab etc. Would love to have this in! |
Atlassian has different APIs for the _Cloud_ and _Server_ (self-hosted) version of Bitbucket. Therefore, a dedicated remote is needed to handle the self-hosted version.
0e4fb24
to
def690b
Compare
No need to start it over, we can use the power of rebase and force pushes 😉. But it's basically somewhat of a start over, haha. First step done, now the Bitbucket implementation picks the Cloud or Server variant based on whether a custom API URL is set. Will get to add in some tests and finish up some missing docs (clippy will probably fail) next. |
Perfect! |
Just lmk when this is ready for review :) |
Definitely! The basics are ready, just didn't get a chance to collect some API calls for some automated tests yet. Once I feel it's ready I'll move it from a draft into a regular PR. |
Just a small heads up, I didn't forget about the PR. Priorities are quite somewhere else at the moment, so I simply didn't get to pull over and sanitize those request/response samples. But I'm sure I'll get to it sometime. |
No worries, sounds good :) |
Atlassian has different APIs for the Cloud and Server (self-hosted)
version of Bitbucket. Therefore, a dedicated remote is needed to handle
the self-hosted version.
Description
Motivation and Context
closes #762
How Has This Been Tested?
Screenshots / Logs (if applicable)
Types of Changes
Checklist: