Conversation
- Update README to document usage of --spec with custom export and template variables. - Add CLI validation to prevent usage of --spec with formats other than custom. - Add/adjust tests to ensure CLI rejects --spec with non-
…s with anything else than --format custom
| data_contract_file: str = None, | ||
| data_contract_str: str = None, | ||
| data_contract: DataContractSpecification = None, | ||
| data_contract: DataContractSpecification | OpenDataContractStandard = None, |
There was a problem hiding this comment.
This change would require a switch in all instance methods, in particular in test() methods.
It feels correct that we should support ODCS spec in the DataContract class. However introducing this in would be a major enhancement (and this PR is a bit specific).
| def export(self, export_format: ExportFormat, model: str = "all", sql_server_type: str = "auto", **kwargs) -> str: | ||
| if export_format == ExportFormat.html or export_format == ExportFormat.mermaid: | ||
| def export(self, export_format: ExportFormat, model: str = "all", spec: Spec = Spec.datacontract_specification, sql_server_type: str = "auto", **kwargs) -> str: | ||
| if export_format == ExportFormat.html or export_format == ExportFormat.mermaid or spec == Spec.odcs: |
There was a problem hiding this comment.
We should enable the new resolve logic (v2) also for ExportFormat.custom. We do not need the spec as a parameter here.
| Optional[Path], | ||
| typer.Option(help="[custom] The file path of Jinja template."), | ||
| ] = None, | ||
| spec: Annotated[ |
There was a problem hiding this comment.
We do not need the parameter, as we can automatically detect whether a YAML file is DCS or ODCS. Supporting a jinja template that can handle DCS and ODCS is not something we want to support. And it could be implemented with an instance of check anyway without this parameter.
|
@gautierc-thelio thanks for your contribution! This is really important, and long overdue. We can simplify the implementation quite a bit, by staying close to how the HTML export itself is implemented. We do not need the |
|
Can you incorporate the requested changes into this PR? |
3 participants
Closes #814