search

datacontract / datacontract-cli

CLI to manage your datacontract.yaml files
https://cli.datacontract.com
Other
352 stars 60 forks source link

fix: load exporters dynamically #293

Closed teoria closed 2 days ago

teoria commented 3 days ago

this PR implements dynamic module load to exporter to avoid errors when the user install a specific dependence like

datacontract-cli[databricks]==0.10.8

jochenchrist commented 3 days ago

Is this ready to merge?

I understand this is necessary, but the code base becomes pretty complex :/

simonharrer commented 3 days ago

I feel that going back to a simple if/else might be much simpler. We can still use the common interface. Thoughts?

teoria commented 2 days ago

@jochenchrist

Is this ready to merge?

I understand this is necessary, but the code base becomes pretty complex :/

Ready to merge.

I don't know how to cover by test this issue( the user don't install the 'avro' and run a AvroImporter)

The dynamic dependency loader add a complexity in this module but allows you to create an external configuration file in the future.

Eg. Config.yaml with the list of supported importers and exporters.

New features (importer or exporters) can be add without any changes in previous code and custom importers and exporters can be add like I wrote in the doc(readme PR).

teoria commented 2 days ago

@simonharrer

I feel that going back to a simple if/else might be much simpler. We can still use the common interface. Thoughts?

I understand your feeling Your are correct If/else is simpler But the main idea of this refactoring is to facilitate growth It's not about creation, it's about maintainability.

This one has become more complex, but all future features will be simpler and we will no longer need to change the core code for each new feature.