Skip to content

Comments

fix: refactor how gitlab title changes are detected#1370

Merged
sudoforge merged 1 commit intomasterfrom
I3bd7fa1c39a9e4dd2176d6e482e30ab68965f6e7
Apr 25, 2025
Merged

fix: refactor how gitlab title changes are detected#1370
sudoforge merged 1 commit intomasterfrom
I3bd7fa1c39a9e4dd2176d6e482e30ab68965f6e7

Conversation

@sudoforge
Copy link
Contributor

@sudoforge sudoforge commented Apr 19, 2025

This change refactors how issue title changes from gitlab events are
detected, fixing an issue (due to upstream changing the format of the
event body from markdown-esque to html), and improving on error
handling.

The error boiled down to a change in the issue title format. Gitlab
changed this on April 17 2025 with the release of version 17.11 0,
although the only place a reference to this change exists is in the
changelog 1, which is not linked to from the releases page.

To account for the potential future in which other fields need to be
parsed in this way, an internal parser library was introduced at
//bridge/gitlab/parser:parser.go with initial support for parsing
title change messages.

An issue was opened with the Gitlab team discussing the fact that this
was a breaking change 2. This may lead to moving title changes (or
maybe all changes) to resource_*_events, which would likely provide a
smoother experience for our use case.

Debugging this issue surfaced a few pain points with this bridge:

  • Errors are few and far between, and when they do exist and are
    managed, they are often not propagated, often existing as simple
    fmt.Printf calls
  • Inconsistent and uninformative logging structure when there are
    errors, leading to challenges in debugging unexpected behavior
  • Fragility: we are parsing random text from event fields (for title
    changes and more). This will likely lead to future breakage should
    Gitlab change the format of other fields. Ideally, the gitlab SDK
    would start classifying notes and have fields like type, old,
    new... but this is unlikely to happen in the short term

Closes: #1367
Change-Id: I3bd7fa1c39a9e4dd2176d6e482e30ab68965f6e7

@sudoforge sudoforge force-pushed the I3bd7fa1c39a9e4dd2176d6e482e30ab68965f6e7 branch 3 times, most recently from 2bb2291 to 5d5d30c Compare April 20, 2025 00:11
@sudoforge
Copy link
Contributor Author

sudoforge commented Apr 20, 2025

i introduced these changes to handle the new title format we're getting from gitlab... unfortunately, as can be seen in the failing import test, it appears that the old markdown-esque format is still returned by their API for older issues.

i'm out of time to work on this for this weekend. i'll be able to pick it back up towards the end of the coming week. if someone wanted to add in an if/else here to handle conditionally using the html or markdown parser and send the patch here (as a comment maybe), i could take a look earlier and merge it. otherwise, we'll just have a broken pipeline for the time being.

@sudoforge
Copy link
Contributor Author

been a little stalled on this trying to get the diff generation working for both gitlab-flavored-markdown and gitlab-flavored-markdown-with-html. it's proving difficult to accurately implement the exact same semantic diff parsing they have, so for now, i'm just going to remove that aspect of the test and hardcode example diffs in like we were doing previously.

This change refactors how issue title changes from gitlab events are
detected, fixing an issue (due to upstream changing the format of the
event body from markdown-esque to html), and improving on error
handling.

The error boiled down to a change in the issue title format. Gitlab
changed this on April 17 2025 with the release of version 17.11 [0],
although the only place a reference to this change exists is in the
changelog [1], which is not linked to from the releases page.

To account for the potential future in which other fields need to be
parsed in this way, an internal parser library was introduced at
`//bridge/gitlab/parser:parser.go` with initial support for parsing
title change messages.

An issue was opened with the Gitlab team discussing the fact that this
was a breaking change [2]. This may lead to moving title changes (or
maybe all changes) to `resource_*_events`, which would likely provide a
smoother experience for our use case.

Debugging this issue surfaced a few pain points with this bridge:

- Errors are few and far between, and when they do exist and are
  managed, they are often not propagated, often existing as simple
  `fmt.Printf` calls
- Inconsistent and uninformative logging structure when there _are_
  errors, leading to challenges in debugging unexpected behavior
- Fragility: we are parsing random text from event fields (for title
  changes and more). This will likely lead to future breakage should
  Gitlab change the format of other fields. Ideally, the gitlab SDK
  would start classifying notes and have fields like `type`, `old`,
  `new`... but this is unlikely to happen in the short term

[0]: https://about.gitlab.com/releases/2025/04/17/gitlab-17-11-released/
[1]: https://gitlab.com/gitlab-org/gitlab/-/commit/b3e1fdcf45f8b18110a2f5217b9964a11616d316#ab09011fa121d0a2bb9fa4ca76094f2482b902b7_5_232
[2]: https://gitlab.com/gitlab-org/gitlab/-/issues/536827

Closes: #1367
Change-Id: I3bd7fa1c39a9e4dd2176d6e482e30ab68965f6e7
@sudoforge sudoforge force-pushed the I3bd7fa1c39a9e4dd2176d6e482e30ab68965f6e7 branch from 5d5d30c to 9205eaf Compare April 25, 2025 15:36
@sudoforge sudoforge marked this pull request as ready for review April 25, 2025 15:38
@sudoforge sudoforge enabled auto-merge April 25, 2025 15:38
@sudoforge sudoforge disabled auto-merge April 25, 2025 15:48
@sudoforge sudoforge merged commit 197eb59 into master Apr 25, 2025
22 of 23 checks passed
@sudoforge sudoforge deleted the I3bd7fa1c39a9e4dd2176d6e482e30ab68965f6e7 branch April 25, 2025 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

import/export to gitlab is broken at HEAD due to upstream changing the message format

1 participant