RFC: Bitbucket support #143

stevealx · 2024-12-19T19:32:18Z

For issue #14

I've tested this against bitbucket.org.

I was only able to test the server/data center support against a prism mock based on the OpenAPI spec. I don't have
an instance to test against, so it may not work in real usage. Hopefully, any fixes will be small.

The README file needs some screen shot updates for adding app passwords. I may be able to sanitize one for Bitbucket Cloud, but a screenshot for the server version will need to come from someone who has it available.

This addresses my personal need, so I don't know how much more time I'm going to spend on it at present. Maybe it will be useful to others, though.

- from: https://www.svgrepo.com/svg/349308/bitbucket - MIT license

- cloud-only at present

- this is pretty ugly - server support has only been tested with a prism mock - watcher counting is not implemented yet

- still needs screen shots and proofreading

brendan-kellam · 2024-12-19T20:11:24Z

Wow this is awesome!! Thanks for the contribution 😃

Will take a look at this later today. Will also take a look at getting a Bitbucket data center setup for testing. Also @gburiola, would you be willing to give this a try?

.gitignore

brendan-kellam · 2024-12-19T20:40:16Z

schemas/v2/index.json

+                "countMisc": {
+                    "type": "boolean",
+                    "default": false,
+                    "description": "Count watchers and forks"


Still reading, but curious - what does this setting do?

I added these counts for parity with the other services, although bitbucket doesn't seem to do stars the way github does.

Against bitbucket cloud I was seeing rate limiting of API calls when watchers and forks were counted for a large number of repos. I made it configurable for the time being, since I don't need the info.

In the server API, I don't see these counts, so they will be zero in that case.

ah kk gotcha - I think we can probably just remove fetching stars / fork count alltogether, at least for now.

For certain hosts (e.g., GitHub), zoekt will use the git config zoekt.github-stars and zoekt.github-forks to influence search rankings. It looks like this isn't plumbed through for bitbucket (via zoekt.bitbucket-stars). If we find there is a need for relevance ranking via stars/forks for bitbucket, then we can do that plumbing then.

brendan-kellam · 2024-12-19T22:48:02Z

packages/backend/src/bitbucket.ts

+                isArchived: project.isArchived,
+                gitConfigMetadata: {
+                    //
+                    // using 'bitbucket-server' lets 'zoekt' generate the correct commit


To confirm, the URL pattern for bitbucket cloud is the same as bitbucket server?
Looking at the zoekt code, it looks like the URL pattern defined is:

// Commit URL: // https://<bitbucketserver-host>/projects/<project>/repos/<repo>/commits/5be7ca73b898bf17a08e607918accfdeafe1e0bc // File URL: // https://<bitbucketserver-host>/projects/<project>/repos/<repo>/browse/<file>?at=5be7ca73b898bf17a08e607918accfdeafe1e0bc

brendan-kellam · 2024-12-19T23:07:14Z

schemas/v2/index.json

+                "countMisc": {
+                    "type": "boolean",
+                    "default": false,
+                    "description": "Count watchers and forks"


ah kk gotcha - I think we can probably just remove fetching stars / fork count alltogether, at least for now.

For certain hosts (e.g., GitHub), zoekt will use the git config zoekt.github-stars and zoekt.github-forks to influence search rankings. It looks like this isn't plumbed through for bitbucket (via zoekt.bitbucket-stars). If we find there is a need for relevance ranking via stars/forks for bitbucket, then we can do that plumbing then.

brendan-kellam · 2024-12-19T23:14:00Z

packages/backend/src/bitbucket.ts

+                if (config.countMisc) {
+                    // not sure the suggested mapping of watchers to stars makes much sense
+                    const { durationMs: starMs, data: foundStars } = await measure(() => client.countWatchers(client, repo));
+                    stars = foundStars
+                    logger.info(`    Found ${stars} watchers for ${repo.name} in ${starMs} ms`);
+                    const { durationMs: forkMs, data: foundForks } = await measure(() => client.countForks(client, repo));
+                    forks = foundForks
+                    logger.info(`    Found ${forks} forks for ${repo.name} in ${forkMs} ms`);
+                }
+


Based on other thread - we can probably just remove this for now

brendan-kellam · 2024-12-20T23:29:45Z

schemas/v2/index.json

+                    "default": false,
+                    "description": "Count watchers and forks"
+                },
+                "url": {


Does it make sense to make this a required field? That way url with be typeof string instead of string | undefined?

brendan-kellam · 2024-12-20T23:30:08Z

packages/backend/src/bitbucket.ts

+//
+function serverClient(config: BitbucketConfig, ctx: AppContext): BitbucketClient {
+    const token = config.token ? getTokenFromConfig(config.token, ctx) : undefined;
+    const url = config.url ? config.url : 'https://junk';


(see other comment about making url required)

brendan-kellam · 2024-12-21T00:33:26Z

packages/backend/src/bitbucket.ts

+        logger.debug('PAGE ' + JSON.stringify(page));
+        if (page.error != null) {
+            // rate limit?
+            await delay(RETRY_TIME);


Is there a way to determine if the error returned was a rate limit vs. some other error? Check status code is 429? Also, had you run into rate limiting issue in your testing?

Currently, this looks like it will enter into a infinite loops if requests never succeed (e.g., if the token is incorrectly scoped and we are hitting 401s)

brendan-kellam · 2024-12-21T00:34:22Z

packages/backend/src/bitbucket.ts

+        logger.debug('PAGE ' + JSON.stringify(page));
+        if (page.error != null) {
+            // rate limit?
+            await delay(RETRY_TIME);


Same rate limit comment as on serverGetPaginatedResult

brendan-kellam · 2024-12-26T07:20:15Z

I didn't get a chance to go over everything since been out the last few days, so just going to release my feedback I had so far.

A general, code quality nit: we should consider making things stronger typed. For example, it would be great if we could type the response bodies s.t., we have more guarantees when we reach for certain properties.

I was hacking around a little bit, and maybe something along these lines this could work

import type { ClientOptions, ClientPathsWithMethod } from "openapi-fetch";
import { createBitbucketCloudClient } from "@coderabbitai/bitbucket/cloud";
import { SchemaProject as CloudProject } from "@coderabbitai/bitbucket/cloud/openapi";

// afaik, this is the only way of extracting the client API type
type CloudAPI = ReturnType<typeof createBitbucketCloudClient>;

// Defines a type that is a union of all API paths that have a GET method in the
// client api.
type CloudGetRequestPath = ClientPathsWithMethod<ClientAPI, "get">;

/**
* Defines a generic response body for any paginated api.
**/
type PaginatedResponse<T> = {
    readonly next?: string;
    readonly page?: number;
    readonly pagelen?: number;
    readonly previous?: string;
    readonly size?: number;
    readonly values?: readonly T[];
}


/**
* We need to do `V extends CloudGetRequestPath` since we will need to call `apiClient.GET(url, ...)`, which
* expects `url` to be of type `CloudGetRequestPath`. See example.
**/
const getPaginatedCloud = async <T, V extends CloudGetRequestPath>(baseUrl: V, get: (url: V) => Promise<PaginatedResponse<T>>) => {
    const results: T[] = [];
    let url = baseUrl;

    while (true) {
        const response = await get(url);

        if (!response.values || response.values.length === 0) { 
            break;
        }

        results.push(...response.values);

        if (!response.next) {
            break;
        }

        // cast required here since response.next is a string.
        url = response.next as V;
    }

    return results;
}

Usage example:

async function cloudGetProjects(client: ClientAPI, workspace: string): Promise<string[]> {
    var results: string[] = [];

    const baseUrl = '/workspaces/{workspace}/projects';

    // `projects` is of type `CloudProject[]`.
    const projects = await getPaginatedCloud<CloudProject, typeof baseUrl>(baseUrl, async (url) => {
        
        // This response body will be typed to `/workspaces/{workspace}/projects`
        const response = await client.GET(url, {
            params: {
                path: {
                    workspace,
                }
            }
        });
        const { data, error } = response;
        if (error) {
            throw new Error (`Failed to fetch projects for workspace ${workspace}: ${error.type}`);
        }
        return data;
    });

    // --> All of this is now typed <--
    for (const project of projects) {
        if (project.key) {
            results.push(project.key);
        }
    }

    logger.debug(`  FOUND ${results.length} PROJECTS IN ${workspace}`);
    return results;
}

There are probably simpler ways of doing this so open to suggestions.

stevealx added 11 commits December 18, 2024 19:52

yarn: add @coderabbitai/bitbucket

3a238d1

ignore: ignore vim swap files

8134f4f

editorconfig: add initial revision

a8d6f60

public: add bitbucket.svg

260e114

- from: https://www.svgrepo.com/svg/349308/bitbucket - MIT license

backend: add bitbucket client

e76c224

- cloud-only at present

schemas: add bitbucket support

447fe6a

main: add bitbucket support

75563c8

web: add bitbucket support

c5fdc88

configs: add sample config for bitbucket cloud

57d55cc

bitbucket: re-factor for cloud and server support

417633d

- this is pretty ugly - server support has only been tested with a prism mock - watcher counting is not implemented yet

README: add bitbucket

433ce9b

- still needs screen shots and proofreading

brendan-kellam reviewed Dec 19, 2024

View reviewed changes

.gitignore Show resolved Hide resolved

brendan-kellam reviewed Dec 19, 2024

View reviewed changes

brendan-kellam reviewed Dec 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFC: Bitbucket support #143

RFC: Bitbucket support #143

stevealx commented Dec 19, 2024

brendan-kellam commented Dec 19, 2024

brendan-kellam Dec 19, 2024

stevealx Dec 19, 2024

brendan-kellam Dec 19, 2024

brendan-kellam Dec 19, 2024 •

edited

Loading

brendan-kellam Dec 19, 2024

brendan-kellam Dec 19, 2024

brendan-kellam Dec 20, 2024

brendan-kellam Dec 20, 2024

brendan-kellam Dec 21, 2024

brendan-kellam Dec 21, 2024

brendan-kellam commented Dec 26, 2024 •

edited

Loading

RFC: Bitbucket support #143

Are you sure you want to change the base?

RFC: Bitbucket support #143

Conversation

stevealx commented Dec 19, 2024

brendan-kellam commented Dec 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brendan-kellam Dec 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brendan-kellam commented Dec 26, 2024 • edited Loading

brendan-kellam Dec 19, 2024 •

edited

Loading

brendan-kellam commented Dec 26, 2024 •

edited

Loading