-
Notifications
You must be signed in to change notification settings - Fork 896
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
Refactor historical / current log collection #17437
Refactor historical / current log collection #17437
Conversation
At one time, I wrote code to fix any_instance_of for newer rubies and it's super brittle and overly magical.
|
||
logfile = LogFile.current_logfile | ||
logfile.update(:file_depot => log_depot, :miq_task => MiqTask.find(taskid)) | ||
post_one_log_pattern("log/*.log", logfile, "Current") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^ this was the goal. It's hard to see in post_historical_logs but there's some "historical log specific" setup and then the call to post_one_log_pattern
.
For an idea of what the automate stuff will look like due to this refactoring:
def post_automate_models(taskid, log_depot)
domain_zip = Rails.root.join("log/domain.zip")
backup_automate_models(domain_zip)
logfile = LogFile.historical_logfile
logfile.update(:file_depot => log_depot, :miq_task => MiqTask.find(taskid))
post_one_log_pattern(domain_zip, logfile, "Models")
ensure
FileUtils.rm_rf(domain_zip)
end
@@ -149,6 +149,68 @@ def stub_vmdb_util_methods_for_collection_log | |||
describe "#post_historical_logs" do | |||
context "Server" do | |||
include_examples "post_[type_of_log]_logs", "MiqServer", :historical | |||
|
|||
context "new tests" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be cleaned up and put in a nicer place in the automate dialog/models export PR
Checked commits jrafanie/manageiq@0881bcf~...76c8e3e with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 app/models/miq_server/log_management.rb
spec/models/miq_server/log_management_spec.rb
|
https://bugzilla.redhat.com/show_bug.cgi?id=1535179 Builds on top of the refactoring of current/historical log collection found in: ManageIQ#17437 LogFile#name was generating a name that was similar but yet different than the destination_file_name. This name is not displayed or used in the UI, so we can push the logic of building the destination_file_name into the name of the LogFile, which is exactly the place we need it to differentiate current/historical/automate models/automate dialogs.
I'll fix up the cops in a followup PR. Many are from the original code and I'd rather not refactor and appease rubocop here. |
https://bugzilla.redhat.com/show_bug.cgi?id=1535179 Builds on top of the refactoring of current/historical log collection found in: ManageIQ#17437 LogFile#name was generating a name that was similar but yet different than the destination_file_name. This name is not displayed or used in the UI, so we can push the logic of building the destination_file_name into the name of the LogFile, which is exactly the place we need it to differentiate current/historical/automate models/automate dialogs.
https://bugzilla.redhat.com/show_bug.cgi?id=1535179 Builds on top of the refactoring of current/historical log collection found in: ManageIQ#17437 LogFile#name was generating a name that was similar but yet different than the destination_file_name. This name is not displayed or used in the UI, so we can push the logic of building the destination_file_name into the name of the LogFile, which is exactly the place we need it to differentiate current/historical/automate models/automate dialogs.
This refactoring is required so we can post things other than historical and current logs such as automate dialogs, domains/models, etc.
https://bugzilla.redhat.com/show_bug.cgi?id=1535179
Note, the tests are going to be moved and refactored in a followup PR when I need to add the automate dialogs/domains posting.