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

[v13] - differentiate discovered resource names #30456

Merged
merged 2 commits into from Sep 11, 2023

Conversation

jentfoo
Copy link
Member

@jentfoo jentfoo commented Aug 14, 2023

v13 Backport of PRs #28845 #30086 and #30054. These PR's build on each other and include refactoring that makes it best to backport them together.

Backports fetcher refactoring from #28845 to branch/v13 without the renaming scheme applied

@jentfoo jentfoo self-assigned this Aug 14, 2023
@github-actions github-actions bot added backport database-access Database access related issues and PRs size/lg labels Aug 14, 2023
@public-teleport-github-review-bot

@jentfoo - this PR will require admin approval to merge due to its size. Consider breaking it up into a series smaller changes.

@jentfoo
Copy link
Member Author

jentfoo commented Aug 14, 2023

I discussed this with @greedy52 in DM. We may not want to bring all these changes in the backport. He is going to help review and dissect this PR.

Copy link
Contributor

@greedy52 greedy52 left a comment

Choose a reason for hiding this comment

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

@GavinFrazar I don't think #28845 should be backported. Could you confirm?

As discussed with @jentfoo , at the mean time, I will try backport the refactoring change separately

@zmb3 zmb3 self-requested a review August 14, 2023 19:30
@greedy52
Copy link
Contributor

backport of #30086:

No conflict for #30054 after this one ⬆️

@GavinFrazar
Copy link
Contributor

GavinFrazar commented Aug 14, 2023

@GavinFrazar I don't think #28845 should be backported. Could you confirm?

As discussed with @jentfoo , at the mean time, I will try backport the refactoring change separately

yes please don't backport the renaming stuff

edit: you can keep the changes from that PR, just remove renaming.go, renaming_test.go and all references to the Apply...Suffix funcs.

edit: applied in 3c5e686

@greedy52
Copy link
Contributor

@GavinFrazar I don't think #28845 should be backported. Could you confirm?
As discussed with @jentfoo , at the mean time, I will try backport the refactoring change separately

yes please don't backport the renaming stuff

edit: you can keep the changes from that PR, just remove renaming.go, renaming_test.go and all references to the Apply...Suffix funcs.

edit: applied in 3c5e686

Thanks for removing the renaming. I think we can either merge this one, or #30461 + #30462

@GavinFrazar
Copy link
Contributor

GavinFrazar commented Aug 15, 2023

@greedy52 how about we merge #30461 and #30462 and then rebase this one onto those, just to pull the refactored fetchers into v13 (to make future backports easier).

edit: or just close this one and i'll do that backport separately. I think it'll be easier to track the backports individually vs one combo backport

* backport #28845 to branch/v13
* remove renaming of discovered resources for backport
@GavinFrazar GavinFrazar changed the title [v13] - Database Service to validate URL of database resources from Discovery… [v13] - differentiate discovered resource names Aug 16, 2023
@jentfoo jentfoo added this pull request to the merge queue Sep 11, 2023
Merged via the queue into branch/v13 with commit 15c993e Sep 11, 2023
19 checks passed
@jentfoo jentfoo deleted the jent/30054_backport-v13 branch September 11, 2023 18:40
@camscale camscale mentioned this pull request Sep 20, 2023
@fheinecke fheinecke mentioned this pull request Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport database-access Database access related issues and PRs size/lg
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants