Skip to content

feat(catalog): always use search for UrlReaderProcessor #28379

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Feb 6, 2025
Merged

feat(catalog): always use search for UrlReaderProcessor #28379

merged 11 commits into from
Feb 6, 2025

Conversation

tcardonne
Copy link
Contributor

@tcardonne tcardonne commented Jan 6, 2025 •
edited
Loading

Hey, I just made a Pull Request!

Copy of PR #27054 that was closed because it was stale. There's some context and discussions in it.

The UrlReaderProccessor now always calls the search method of the UrlReaders. Previous behavior was to call the search method only if the parsed Git URL's filename contained a wildcard and use readUrl otherwise. UrlReaderService must implement this logic in the search method instead.

This allows each UrlReaderService implementation to check whether it's a search URL (that contains a wildcard pattern) or not using logic that is specific to each provider.

In the different UrlReadersService, the search method have been updated to use the readUrl if the given URL doesn't contain a pattern.
For UrlReaders that didn't implement the search method, readUrl is called internally.

See comment by @Rugvip for the reasoning: #27054 (comment)

Closes #22504

Unblocks #25704

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)
  • All your commits have a Signed-off-by line in the message. (more info)

@tcardonne tcardonne requested review from a team and backstage-service as code owners January 6, 2025 15:31
@github-actions github-actions bot added the area:catalog Related to the Catalog Project Area label Jan 6, 2025
@backstage-goalie
Copy link
Contributor

backstage-goalie bot commented Jan 6, 2025 •
edited
Loading

Important

This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior.

Changed Packages

Package Name Package Path Changeset Bump Current Version
@backstage/backend-defaults packages/backend-defaults patch v0.8.0-next.1
@backstage/create-app packages/create-app patch v0.5.25-next.1
@backstage/plugin-catalog-backend plugins/catalog-backend minor v1.31.0-next.1

vinzscam
vinzscam requested changes Jan 15, 2025
View reviewed changes
@tcardonne tcardonne force-pushed the feat-urlreader-issearchurl branch from 4fac26f to d8484a9 Compare January 15, 2025 12:54
@tcardonne tcardonne requested a review from vinzscam January 15, 2025 12:55
@tcardonne tcardonne force-pushed the feat-urlreader-issearchurl branch from d8484a9 to 298b231 Compare January 15, 2025 12:55
@tcardonne
Copy link
Contributor Author

Hi @Rugvip @vinzscam,

Is there anything I can do to move this forward? I wasn't able to attend the last framework SIG to talk about this PR 😞

I'd like to start adapting #28387 on top of this one but I can't start yet because we use it to patch our instance. Also it's becoming hard for us to keep them without conflicts and it makes the upgrading process a bit hard (as we have to patch 4 packages)

Thanks!

vinzscam
vinzscam requested changes Jan 22, 2025
View reviewed changes
Copy link
Member

@vinzscam vinzscam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apologies for the waiting time @tcardonne. Let's try to ship this asap

@tcardonne
feat(catalog): always use search for UrlReaderProcessor
3740229 
Signed-off-by: Thomas Cardonne <thomas.cardonne@adevinta.com>
@tcardonne
feat: add config flag
564170b 
Signed-off-by: Thomas Cardonne <thomas.cardonne@adevinta.com>
@tcardonne
fix: remove breaking change from changeset
3d38382 
Signed-off-by: Thomas Cardonne <thomas.cardonne@adevinta.com>
@tcardonne tcardonne force-pushed the feat-urlreader-issearchurl branch from 58008fa to bcd6f2e Compare January 22, 2025 22:22
@tcardonne
feat: throw if given a search url on awsS3, awsCodeCommit, fetch, ger…
f5e3c09 
…rit, gitea and harness url readers

Signed-off-by: Thomas Cardonne <thomas.cardonne@adevinta.com>
@tcardonne tcardonne force-pushed the feat-urlreader-issearchurl branch from bcd6f2e to f5e3c09 Compare January 22, 2025 22:36
@tcardonne tcardonne requested a review from vinzscam January 23, 2025 08:45
vinzscam
vinzscam approved these changes Jan 27, 2025
View reviewed changes
Copy link
Member

@vinzscam vinzscam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks good @tcardonne 👏

Rugvip
Rugvip requested changes Jan 27, 2025
View reviewed changes
Copy link
Member

@Rugvip Rugvip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome stuff! 🎉 👌

Just a few small nits and we're good to ship

@tcardonne
fix: separate changesets for backend-defaults and plugin-catalog-backend
5aebc13 
Signed-off-by: Thomas Cardonne <thomas.cardonne@adevinta.com>
@tcardonne
feat: use mockServices.rootConfig
d1cc122 
Signed-off-by: Thomas Cardonne <thomas.cardonne@adevinta.com>
@tcardonne
feat: add useUrlReadersSearch in default app-config
2a98565 
Signed-off-by: Thomas Cardonne <thomas.cardonne@adevinta.com>
@tcardonne tcardonne requested a review from Rugvip January 27, 2025 15:17
@tcardonne
Copy link
Contributor Author

@Rugvip I resolved the few nits you mentioned, thanks for the feedback 🙏

There's failures in the CI but it doesn't seem related to this PR 🤔

@tcardonne
fix: typo in gerrit error message
c1fed63 
Signed-off-by: Thomas Cardonne <thomas.cardonne@adevinta.com>
@tcardonne tcardonne mentioned this pull request Feb 3, 2025
Merged
5 tasks
@tcardonne
fix: add changeset for create-app
bfa4ea9 
Signed-off-by: Thomas Cardonne <thomas.cardonne@adevinta.com>
@tcardonne tcardonne force-pushed the feat-urlreader-issearchurl branch from 8c506e2 to bfa4ea9 Compare February 3, 2025 17:59
Rugvip
Rugvip approved these changes Feb 6, 2025
View reviewed changes
Copy link
Member

@Rugvip Rugvip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome stuff, thank you! 😁 🎉

:shipit:

@Rugvip Rugvip merged commit fccbb7f into backstage:master Feb 6, 2025
29 checks passed
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 6, 2025

Thank you for contributing to Backstage! The changes in this pull request will be part of the 1.36.0 release, scheduled for Tue, 18 Feb 2025.

@tcardonne tcardonne mentioned this pull request Feb 28, 2025
Merged
5 tasks
@tcardonne tcardonne mentioned this pull request Apr 29, 2025
Merged
5 tasks
@Mohamed-E Mohamed-E mentioned this pull request May 5, 2025
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rugvip Rugvip Rugvip approved these changes

@vinzscam vinzscam vinzscam approved these changes

@backstage-service backstage-service Awaiting requested review from backstage-service backstage-service is a code owner

@benjdlambert benjdlambert Awaiting requested review from benjdlambert benjdlambert is a code owner automatically assigned from backstage/maintainers

Assignees
No one assigned
Labels
area:catalog Related to the Catalog Project Area
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support Wildcards for all integrations in UrlReaderProcessor
3 participants
@tcardonne @Rugvip @vinzscam