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

[WIP] Remove DatabaseBackup/FileDepot views #7816

Closed

Conversation

@miq-bot
Copy link
Member

miq-bot commented Aug 10, 2021

Checked commit NickLaMuro@e88869e with ruby 2.6.3, rubocop 1.13.0, haml-lint 0.35.0, and yamllint
9 files checked, 1 offense detected

**

  • 💣 💥 🔥 🚒 - Linter/Haml - error parsing JSON result

@miq-bot miq-bot added the wip label Aug 10, 2021
@NickLaMuro
Copy link
Member Author

Closing for the same reason as ManageIQ/manageiq#21360 was:

ManageIQ/manageiq#21360 (comment)

@NickLaMuro NickLaMuro closed this Aug 17, 2021
@NickLaMuro
Copy link
Member Author

NickLaMuro commented Aug 31, 2021

@Fryguy So I deleted closed this because I was thinking that ManageIQ/manageiq#21360 meant that we couldn't touch FileDepot models at all.

But now that I think about it again, is there anything wrong with deleting the FileDepot views? We need to keep the models around for the PXE servers, but aside from that, do we need the views that I deleted as part of this PR? If not, I will re-open this PR as is instead of creating a new one that just deletes the DatabaseBackup code.

@Fryguy
Copy link
Member

Fryguy commented Aug 31, 2021

Right, the goal would be to remove the "callers" first, so dropping the views is removing one of the callers. I did a quick review and I think everything is just removing callers, so this is probably ok to reopen.

@NickLaMuro
Copy link
Member Author

NickLaMuro commented Aug 31, 2021

@Fryguy Oh, what I meant was that FileDepot specifically was off limits because of:

ManageIQ/manageiq#21360 (comment)

However, the FileDepot code in question is just used internally for the models, correct? Or do you first have to create a FileDepot in the ops UI, then add a PXE server that uses that? I haven't used this functionality before, so I am uncertain myself.

@Fryguy
Copy link
Member

Fryguy commented Sep 1, 2021

I'm not certain - I thought it was automatic under the covers for PXE Server, but perhaps @bdunne knows

@kbrock
Copy link
Member

kbrock commented Sep 21, 2021

@NickLaMuro So can we reopen and merge this one?

I know we didn't get to delete all things but at least we got to clean out a lot here and the other dependent PRs

@NickLaMuro
Copy link
Member Author

@kbrock The question asked was if I need to make a fresh PR that removes only the DB backup views, or if the FileDepot views are only needed by Logging/DB Backup code and can be removed. If we can get rid off both, that would be great, but this isn't code I am familiar with an can comment on.

@bdunne re-ping for #7816 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants