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

Add support for exporting and importing generic object definitions #18688

Merged
merged 4 commits into from
May 2, 2019

Conversation

romanblanco
Copy link
Member

@romanblanco romanblanco commented Apr 25, 2019

These rake scripts and classes provide functionality for exporting/importing of the Generic Object Definitions.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1595259

Assuming the generic object definitions are all created by user, and therefore there are no default / custom generic object definitions.
For that reason, we don't work with --all argument as can be seen in other ManageIQ exportable objects.

While importing, the overwrite switch defaults to true. To disable overwriting of imported objects use --no-overwrite switch.

Links

Steps for Testing/QA

Exporting

Create a directory for the exports

mkdir tmp/generic_object_definitions

Export user defined

bin/rake evm:export:generic_object_definitions -- --directory tmp/generic_object_definitions

Importing

  1. Import all generic object definition yaml files in a directory
bin/rake evm:import:generic_object_definitions -- --source tmp/generic_object_definitions
  1. Import a specific generic object definition yaml file
bin/rake evm:import:generic_object_definitions -- --source tmp/generic_object_definitions/god_runtime_error.yml

@romanblanco
Copy link
Member Author

@miq-bot add_label enhancement tools
/cc @gmcculloug @gtanzillo

@miq-bot
Copy link
Member

miq-bot commented Apr 25, 2019

@romanblanco Cannot apply the following label because they are not recognized: enhancement tools

---
- GenericObjectDefinition:
name: Apep
description: Ancient Egyptian deity who embodied chaos
Copy link
Member

Choose a reason for hiding this comment

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

LOOOL. I love this.

@@ -21,6 +23,7 @@ class GenericObjectDefinition < ApplicationRecord
REG_ATTRIBUTE_NAME = /\A[a-z][a-zA-Z_0-9]*\z/
REG_METHOD_NAME = /\A[a-z][a-zA-Z_0-9]*[!?]?\z/
ALLOWED_ASSOCIATION_TYPES = (MiqReport.reportable_models + %w(GenericObject)).freeze
IMPORT_CLASS_NAMES = %w(GenericObjectDefinition).freeze
Copy link
Member

Choose a reason for hiding this comment

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

Is this used anywhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

@Fryguy
Copy link
Member

Fryguy commented Apr 26, 2019

@gtanzillo Please review.

Copy link
Member

@gmcculloug gmcculloug left a comment

Choose a reason for hiding this comment

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

@eclarizio can you help review this one?

@@ -18,7 +18,7 @@
before do
@user_admin = FactoryBot.create(:user_admin)
report_string = MiqReport.export_to_yaml([@old_report.id], MiqReport)
@new_report = YAML.load(report_string).first
@new_report = YAML.load(report_string).first["MiqReport"]
Copy link
Member

Choose a reason for hiding this comment

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

These report changes should be moved to a separate PR to keep this GenericObject focused.

end
end

describe "when the source file modifies an existing report" do
Copy link
Member

Choose a reason for hiding this comment

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

Several references to report here which seem like remnants from copy/paste.

@romanblanco romanblanco force-pushed the bz1595259 branch 2 times, most recently from fe74675 to 0278221 Compare April 29, 2019 13:03
Copy link
Member

@eclarizio eclarizio left a comment

Choose a reason for hiding this comment

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

The code seems fine, but you could probably separate the new methods in GenericObjectDefinition into a mixin since that seems to be the pattern we have for MiqWidget, MiqReport, and MiqPolicy. They all just have an ImportExport module with at least the import_from_hash and export_to_array methods in them, and then the object in question just does include_concern 'ImportExport'.

@miq-bot
Copy link
Member

miq-bot commented Apr 30, 2019

Checked commits romanblanco/manageiq@fc78b82~...77e1879 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
7 files checked, 0 offenses detected
Everything looks fine. 🏆

Copy link
Member

@eclarizio eclarizio left a comment

Choose a reason for hiding this comment

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

👍

@gmcculloug gmcculloug requested a review from d-m-u May 2, 2019 11:47
@d-m-u
Copy link
Contributor

d-m-u commented May 2, 2019

Just fyi the description of the commands looks copypasted with an error:
bin/rake evm:export:generic_object_definitions -- --directory tmp/generic_object_definitions
and your import command is
bin/rake evm:export:generic_object_definitions -- --directory /tmp/generic_object_definitions

(you should probably take the slash off the import command line in the description)

@gmcculloug gmcculloug merged commit da7225f into ManageIQ:master May 2, 2019
@gmcculloug gmcculloug added this to the Sprint 111 Ending May 13, 2019 milestone May 2, 2019
@romanblanco romanblanco deleted the bz1595259 branch May 2, 2019 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants