Added ADR document describing why the notion of dialects was introduced in the common sql provider #45456
Conversation
…ects was introduced in the common sql provider
|
NIce! I think we are also missing user facing docs about what dialects are and how to use them (ADR is for Airflow developers to document our design choices but it's not really a user facing doc) |
Where should I put those user facing docs? Is that under docs/apache-airflow-providers-common-sql? Should I create a new dialects.rst document? |
Yes. You dont have to create new .rst |
…der documentation
|
@dabla do you intend to add the docs in this PR? |
| At the moment there are only 3 dialects available: | ||
|
|
||
| - ``default`` :class:`~airflow.providers.common.sql.dialects.dialect.Dialect` reuses the generic functionality that was already available in the :class:`~airflow.providers.common.sql.hooks.sql.DbApiHook`; | ||
| - ``mssql`` :class:`~airflow.providers.microsoft.mssql.dialects.mssql.MsSqlDialect` specialized for Microsoft SQL Server; | ||
| - ``postgresql`` :class:`~airflow.providers.postgres.dialects.postgres.PostgresDialect` specialized for PostgreSQL; | ||
|
|
||
| The dialect to be used will be derived from the connection string, which sometimes won't be possible. There is always | ||
| the possibility to specify the dialect name through the extra options of the connection: | ||
|
|
||
| .. code-block:: | ||
|
|
||
| dialect_name: 'mssql' |
There was a problem hiding this comment.
This is nice!
I think we better also have dialects.rst in the docs of mssql and postgres and reference the guide from here.
Some users may land directly in the relevant docs rather than in the common.sql doc
There was a problem hiding this comment.
I will look at it shortly. Been kinda busy with refactors and stuff :)
There was a problem hiding this comment.
@dabla can you add dialects.rst also in mysql and postgres providers?
There was a problem hiding this comment.
@dabla can you add
dialects.rstalso in mysql and postgres providers?
I've added the reference to dialects in both indexes.
There was a problem hiding this comment.
I don't think this can pass the doc build... i think you have to create a separate doc for each one but if the doc build passes I am fine with it
There was a problem hiding this comment.
Sorry, "late to the party" now digging through my backlog... wanted to review earlier.
I really like this and the description.
For the docs build problem I see two options: (1) [not sure if this really works] outside the TOC create a small RST just with the heading and pointing to the other doctree - or (2) if you want to have exactly the same doc in each provider then use a symlink in GIT such that the RST is maintained in one place and each doctree has the same. (As long as we not move the providers the symlink is working in Sphinx)
Can you add some example the the dialects.rst or is this already contained in the implementation PR?
Will look into it. I also opened another PR related to this which fixes an issue with reserved words and special characters using the dialects. |
Not that found of symlinks, dunno how git will handle those. Aren't http references an option? I saw it's also used within rst. For example (won't exist atm ofc): https://airflow.apache.org/docs/apache-airflow-providers-common-sql/stable/dialects.html |
Added ADR document describing why the notion of dialects was introduced in the common sql provider.
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.