Skip to content
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

Change script in **UrlReaderProcessor.ts** adding some code to handle URL Reader from GCS with wildcard /* #22141

Merged
merged 6 commits into from
Jan 16, 2024

Conversation

armandocomellas1
Copy link
Contributor

@armandocomellas1 armandocomellas1 commented Jan 8, 2024

This fixes bug #21955

✔️ Checklist

  • ChangeSet name is tasty-tips-itch.md and there l describe what it doing it the code. (more info)
  • 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)

The implementation consist in adding a path with wildcard starting with https://storage.cloud.google.com/[bucketname]/*
and the wilcard is to load all the files inside the bucket and not just a single one with the full path

So the behavior is:

Before implementation we had this:
Captura de pantalla 2024-01-08 130651

After implementarion:
Captura de pantalla 2024-01-08 130618

So as you can see after the implementation the UI is capable to render all the components loaded from a relative path with wildcard.

Thank you for reviewing the pull request.

@armandocomellas1 armandocomellas1 requested review from a team as code owners January 8, 2024 19:08
@github-actions github-actions bot added the area:catalog Related to the Catalog Project Area label Jan 8, 2024
@backstage-goalie
Copy link
Contributor

backstage-goalie bot commented Jan 8, 2024

Changed Packages

Package Name Package Path Changeset Bump Current Version
@backstage/plugin-catalog-backend plugins/catalog-backend patch v1.16.1-next.2

@armandocomellas1 armandocomellas1 changed the title Change script in **UrlReaderProcessor.ts** adding some code to handle URL Reader from GCS with wildcard * This fixes bug https://github.com/backstage/backstage/issues/21955 Jan 8, 2024
@armandocomellas1 armandocomellas1 changed the title This fixes bug https://github.com/backstage/backstage/issues/21955 Change script in **UrlReaderProcessor.ts** adding some code to handle URL Reader from GCS with wildcard /* Jan 8, 2024
@armandocomellas1
Copy link
Contributor Author

Hi All!

Just for your knowledge, my PR is failling in just two test that are nothing to do with the change l made.

failing test: E2E Linux / E2E Linux 18.x (pull_request)
E2E Linux / E2E Linux 20.x (pull_request)

@jamieklassen
Copy link
Member

Hi All!

Just for your knowledge, my PR is failling in just two test that are nothing to do with the change l made.

failing test: E2E Linux / E2E Linux 18.x (pull_request) E2E Linux / E2E Linux 20.x (pull_request)

Yep, those will stay broken until mui/material-ui#40427 is fixed

@armandocomellas1
Copy link
Contributor Author

Hey guys 👋,

I just want to follow up this open issue PR, l have replaced the line const { filepath } = parseGitUrl(location); with const { pathname: filepath } = new URL(location); looks like it has worked.

Let me know your comments.

Thank you for taking the time to review them

@jamieklassen @benjdlambert @viglesiasce

@benjdlambert
Copy link
Member

@armandocomellas1 did you mean to keep all the other changes to the other files that you've made like the GKEClusterLocator.ts in kubernetes-backend? Or are you just looking to bug #21955?

I think that you might want to revert the other changes as they don't seem relevant for this bug fix, maybe you can explain what these other changes do in another PR and what issue they are trying to solve as I don't see the overlap here.

Thanks!

@armandocomellas1
Copy link
Contributor Author

@armandocomellas1 did you mean to keep all the other changes to the other files that you've made like the GKEClusterLocator.ts in kubernetes-backend? Or are you just looking to bug #21955?

I think that you might want to revert the other changes as they don't seem relevant for this bug fix, maybe you can explain what these other changes do in another PR and what issue they are trying to solve as I don't see the overlap here.

Thanks!

Yeah, that's my bad, l mixed both issue solution in this PR, l had in my local repo the solution for the Gke backend auth, but they are two completley separetly things, if you want l can close this issue, and l will open a new PR for each issue.

Thanks!

Signed-off-by: armandocomellas1 <cgarmando@google.com>
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Jan 12, 2024
armandocomellas1 added 2 commits January 12, 2024 11:36
…eset thirty-cats-help.md

Signed-off-by: armandocomellas1 <cgarmando@google.com>
…eset thirty-cats-help.md

Signed-off-by: armandocomellas1 <cgarmando@google.com>
@armandocomellas1
Copy link
Contributor Author

Hi Guys!

@benjdlambert @jamieklassen
I already reverted to just the single and about this issue.

Please if you can review l will be happy to hear any news.

Thanks!

Copy link
Member

@benjdlambert benjdlambert left a comment

Choose a reason for hiding this comment

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

Just a small comment on the changeset. Do you think you could also add a test for this too?

.changeset/thirty-cats-help.md Outdated Show resolved Hide resolved
@benjdlambert
Copy link
Member

Handing over to @backstage/catalog-maintainers after the suggestions are done.

…st for new URL method

Signed-off-by: armandocomellas1 <cgarmando@google.com>
Copy link
Member

@benjdlambert benjdlambert left a comment

Choose a reason for hiding this comment

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

NIce! Thanks for this! I just updated the test a little bit because I would like to get this in the release today 🎉

Copy link
Contributor

github-actions bot commented Jan 16, 2024

Uffizzi Cluster pr-22141 was deleted.

@benjdlambert benjdlambert merged commit a1762c3 into backstage:master Jan 16, 2024
25 of 27 checks passed
Copy link
Contributor

Thank you for contributing to Backstage! The changes in this pull request will be part of the 1.22.0 release, scheduled for Tue, 16 Jan 2024.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:catalog Related to the Catalog Project Area
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants