BitBucket implementation + webhook events / integration with Studio

[duijf]

Hey CML team! As you've probably noticed, I'm working on the integration between Studio + CML PR comments on BitBucket.

I'm running into a limitation with BitBucket webhooks and wanted to check if you are open to changing the CML BitBucket implementation to make the integration work more reliably.

Problem: pullrequest:comment_updated is very unreliable

BitBucket does not send webhook events for pullrequest:comment_updated in a lot of scenario's.

From the BB docs:

If a user updates the same comment with not much time in between, Bitbucket only sends the event request the first time the comment is updated. However, if some time passes and that user updates the comment again, Bitbucket sends the event request a second time.

From my experimentation, "not much time in between" can mean "not even after 20 minutes" (!). I have as-of-yet not been able to make BitBucket send me a single comment updated event¹. (Not when editing a PR comment with CML, but also not when editing manually).

Update: I've edited a comment I've posted after 3 days, and BitBucket still did not send any update payload. Seems like this is just plain broken.

How CML performs edits

When cml send-comment is invoked with the --pr and --update flags, CML checks if a PR comment exists and updates it if this is the case. The update contains the new contents of the report.

This update happens by sending a PUT request to BitBucket:

cml/src/drivers/bitbucket_cloud.js

Lines 154 to 166 in 0a674fe

	async prCommentUpdate(opts = {}) {
	const { projectPath } = this;
	const { report, prNumber, id } = opts;
	const endpoint = `/repositories/${projectPath}/pullrequests/${prNumber}/comments/${id}`;
	const output = await this.request({
	endpoint,
	method: 'PUT',
	body: JSON.stringify({ content: { raw: report } })
	});
	return output.links.self.href;
	}

Why Studio needs to know about updates

When CML updates a PR comment, the new comment may contain a new SHA. In these cases, Studio needs to associate the CML report with a different commit and display it accordingly. We are therefore interested in knowing when things change.

Options to resolve this

1. In theory, Studio could periodically poll BitBucket to see if any CML reports have been updated. I'd really like to avoid this though, as providing these updates to users in a timely manner would take up quite a sizeable portion of the API limits.
2. Make CML delete old BitBucket reports when invoked with --pr and --update.
3. Provide a different flag (e.g. --delete / --delete-old) and tell users to use this if they want smooth integration with Studio.
4. Don't do anything special / tell users not to use --update if they want to use CML with Studio

Out of all these options, I'm inclined to lean towards --delete / --delete-old (name up for discussion ofc.):

• The flag could be useful to other users as well, if they want notifications every time a new CML report is available, but don't care about the old ones any longer.
• Some CML users may use BitBucket and not use Studio and appreciate the current behavior of --edit.

What do you think about the options I posted? Anything I missed?

Footnotes

1. I triple checked that I'm not doing anything weird. The webhook I have is configured to send updates when PR comments are edited. The created+deleted events work fine. ↩

[casperdcl]

I'd prefer (4)-ish: document caveats of --update in BB under https://cml.dev/doc/ref/send-comment#faqs-and-known-issues

[DavidGOrtega]

@duijf sorry to hear... if have also found some quirks with the api like intermittent failures

[duijf]

Update: turns out GitLab has even more limitations than BitBucket around webhook events about comments. GL only provides webhook events for comment creation (either PR or commit comments). Editing and deletion events are not exposed, so the --delete flag I proposed in option 3) wouldn't work there. Here is a feature request for more detailed webhook events in GitLab: https://gitlab.com/gitlab-org/gitlab/-/issues/17057

Since option 3) wouldn't work for GitLab, I guess we can shelve it to avoid a bunch of special cases for different SCM providers. For now, documentation for this behavior is probably good enough until a customer runs into these limitations and wants us to do something about them.

If we ever want to fix this, these options are left:

• Make Studio poll GH/GL/BB periodically for updates to CML reports that we track.
• New, just thought of this: Introduce a separate webhook in Studio that CML could send requests to to let Studio know something has changed on the SCM provider.

[dacbd]

• New, just thought of this: Introduce a separate webhook in Studio that CML could send requests to to let Studio know something has changed on the SCM provider.

That would have some security implications and would be non-trivial to implement properly imo

[duijf]

Yup, it would definitely be a bit involved. Just left it as an idea that we can get to if it turns out that people really need it :)

[DavidGOrtega]

Introduce a separate webhook in Studio that CML could send requests

@duijf a webhook or a rest api call? The later its very doable

[0x2b3bfa0]

Closing in favor of https://github.com/iterative/studio/issues/3753

[casperdcl]

reopening since https://github.com/iterative/studio/issues/3753 isn't a thing :)