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

Converter config only really works with GenericConverter #40

Open
jdreesen opened this issue Feb 5, 2023 · 2 comments
Open

Converter config only really works with GenericConverter #40

jdreesen opened this issue Feb 5, 2023 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@jdreesen
Copy link
Member

jdreesen commented Feb 5, 2023

The bundle config currently allows you to specify a custom converter via neusta_converter.converter.<name>.converter: <class-name>, but this only works if it has exactly the same constructor parameters as the GenericConverter, which is not the case with the StrategicConverter.

One solution would be to introduce a new level for the converter type and move the converter-specific configuration below it.

So, instead of neusta_converter.converter.<name>... it would be neusta_converter.converter.<name>.<type>....

@jdreesen jdreesen added the bug Something isn't working label Feb 5, 2023
@jdreesen jdreesen self-assigned this Feb 5, 2023
@mike4git
Copy link
Collaborator

What is the argument against extending the configuration to something e.g.:
neusta_converter.strategy_converter.<name>

AFAIK there will be no more other types of conversions but:

  • using Factory and Populators
  • find a strategy (which is perhaps using Factory and Populators)

Wouldn't that make backward compatibility easier and nevertheless allow strategy converters?

@jdreesen
Copy link
Member Author

jdreesen commented Apr 4, 2024

I wouldn't be so sure that there will only ever be exactly these two converter types.

I would also like to have the option of extending the configuration from other bundles or the project.

The proposal in #41 is not a big change in the (YAML) config, and projects should be able to adapt quickly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants