WIP: BulkImports - Replace `extractor` and `loader` DSL by Pipeline methods
What does this MR do?
This is a simple refactoring, basically moving code around. The main reasons for this proposal are:
- The DSL was changed to support only 1
extractorand 1loader - Simplify the DSL means less supporting code to maintain
- Optional indirection: with the DSL, extractors and loaders need to be classes. Using methods we can move this logic to within the Pipeline, but with the flexibility to extract to external classes when needed.
- The
after_runstep, use to be a DSL, was recently changed to be a method as well, so it's a step towards the same direction. - (Personal flavoring) IMHO, more code within the Pipeline itself makes it easier to understand, since one can see almost all the steps in the same file.
Extra:
- Since we don't have specific classes for the extractors/loaders I change the log message to show, instead, the step of the pipeline.
Before the change the log was like:
{"severity":"INFO","time":"2021-01-21T15:28:27.374Z","correlation_id":null,"bulk_import_entity_id":37,"bulk_import_entity_type":"group_entity","loader":"EE::BulkImports::Groups::Loaders::EpicsLoader"}
{"severity":"INFO","time":"2021-01-21T15:28:30.585Z","correlation_id":null,"bulk_import_entity_id":37,"bulk_import_entity_type":"group_entity","extractor":"BulkImports::Common::Extractors::GraphqlExtractor"}
Now it's like
{"severity":"INFO","time":"2021-01-21T21:54:35.326Z","correlation_id":null,"bulk_import_entity_id":65,"bulk_import_entity_type":"group_entity","pipeline_class":"EE::BulkImports::Groups::Pipelines::EpicsPipeline","step":"loader"}
{"severity":"INFO","time":"2021-01-21T21:54:35.764Z","correlation_id":null,"bulk_import_entity_id":64,"bulk_import_entity_type":"group_entity","pipeline_class":"EE::BulkImports::Groups::Pipelines::EpicsPipeline","step":"extractor"}
Screenshots (strongly suggested)
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec -
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team
Edited by Kassio Borges