Skip to content

FIX: map vertica airflow connection schema for vertica database#741

Merged
tatiana merged 9 commits into
astronomer:mainfrom
perttus:fix/vertica-airflow-param-mapping
Dec 13, 2023
Merged

FIX: map vertica airflow connection schema for vertica database#741
tatiana merged 9 commits into
astronomer:mainfrom
perttus:fix/vertica-airflow-param-mapping

Conversation

@perttus
Copy link
Copy Markdown
Contributor

@perttus perttus commented Dec 4, 2023

Description

Map airflow connection schema for vertica database to keep it consistent with other connection types and profiles.
Also Vertica Airflow provider assumes this:
https://github.com/apache/airflow/blob/395ac463494dba1478a05a32900218988495889c/airflow/providers/vertica/hooks/vertica.py#L72

Related Issue(s)

closes: #740
related: #538

Checklist

  • I have added tests that prove my fix is effective or that my feature works

@perttus
map airflow connection schema for vertica database
00ea858 
Signed-off-by: Perttu Salonen <perttu.salonen@nitor.com>
@perttus perttus requested a review from a team as a code owner December 4, 2023 10:26
@perttus perttus requested a review from a team December 4, 2023 10:26
@dosubot dosubot Bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Dec 4, 2023
@netlify
Copy link
Copy Markdown

netlify Bot commented Dec 4, 2023 •
edited
Loading

👷 Deploy Preview for amazing-pothos-a3bca0 processing.

Name Link
🔨 Latest commit bdd4d0a
🔍 Latest deploy log https://app.netlify.com/sites/amazing-pothos-a3bca0/deploys/6579b1d652b81d00087bf5b9

@dosubot dosubot Bot added area:profile Related to ProfileConfig, like Athena, BigQuery, Clickhouse, Spark, Trino, etc profile:vertica Related to Vertica ProfileConfig labels Dec 4, 2023
@perttus perttus changed the title FIX: map airflow connection schema for vertica database FIX: map vertica airflow connection schema for vertica database Dec 4, 2023
tatiana
tatiana reviewed Dec 4, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@tatiana tatiana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@perttus Thanks for making this consistent!

I've seen a similar issue with Airflow Postgres connection, when Airflow incorrectly named Postgres schema as database: (https://github.com/apache/airflow/blob/0953e0f844fa5db81c2b461ec2433de1935260b3/airflow/providers/postgres/hooks/postgres.py#L138). To fix this will be a breaking change. I didn't realise the Vertica Airflow connection had the same issue.

I just checked the tests, and it seems we're covering scenarios when the user uses profile_args to define the actual schema - so we should be safe. Do you recommend any documentation about Vertica database/schema?

tatiana
tatiana reviewed Dec 4, 2023
View reviewed changes
Comment thread cosmos/profiles/vertica/user_pass.py
@tatiana tatiana added this to the 1.3.0 milestone Dec 4, 2023
@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 4, 2023 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8809e46) 93.18% compared to head (bdd4d0a) 93.18%.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main     #741   +/-   ##
=======================================
  Coverage   93.18%   93.18%           
=======================================
  Files          55       55           
  Lines        2451     2451           
=======================================
  Hits         2284     2284           
  Misses        167      167

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@perttus
Copy link
Copy Markdown
Contributor Author

perttus commented Dec 7, 2023

@perttus Thanks for making this consistent!

I've seen a similar issue with Airflow Postgres connection, when Airflow incorrectly named Postgres schema as database: (https://github.com/apache/airflow/blob/0953e0f844fa5db81c2b461ec2433de1935260b3/airflow/providers/postgres/hooks/postgres.py#L138). To fix this will be a breaking change. I didn't realise the Vertica Airflow connection had the same issue.

I just checked the tests, and it seems we're covering scenarios when the user uses profile_args to define the actual schema - so we should be safe. Do you recommend any documentation about Vertica database/schema?

Yes, I noticed the same. Seems to be the case also for Redshift and Exasol. It kind of makes sense when it's not really necessary to define the schema when one simply needs to connect to the database. It's a pity that airflow connection doesn't have a database field so it would become more consistent. I guess this is how it should be stated also in the documentation.

@perttus
added explanation for the mapping
0a5e4a8 
Signed-off-by: Perttu Salonen <perttu.salonen@nitor.com>
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Dec 7, 2023
jbandoro
jbandoro reviewed Dec 7, 2023
View reviewed changes
Comment thread cosmos/profiles/vertica/user_pass.py Outdated
@perttus
Revert "added explanation for the mapping"
c9d2565 
This reverts commit 0a5e4a8.
@perttus
move docstring with a sphinx note under the class definition
f1f82fb 
Signed-off-by: Perttu Salonen <perttu.salonen@nitor.com>
tatiana
tatiana approved these changes Dec 13, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@tatiana tatiana left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the feedback, @perttus ! We'll merge this CR once the checks pass.

@tatiana tatiana added the lgtm This PR has been approved by a maintainer label Dec 13, 2023
@tatiana tatiana merged commit 4369987 into astronomer:main Dec 13, 2023
@perttus perttus deleted the fix/vertica-airflow-param-mapping branch December 14, 2023 07:57
@tatiana tatiana mentioned this pull request Jan 4, 2024
Merged
tatiana added a commit that referenced this pull request Jan 4, 2024
@tatiana
Release 1.3.0 (#781)
0de8bdd 
**Features**

* Add new parsing method ``LoadMode.DBT_LS_FILE`` by @woogakoki in #733
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/parsing-methods.html#dbt-ls-file)).
* Add support to select using (some) graph operators when using
``LoadMode.CUSTOM`` and ``LoadMode.DBT_MANIFEST`` by @tatiana in #728
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/selecting-excluding.html#using-select-and-exclude))
* Add support for dbt ``selector`` arg for DAG parsing by @jbandoro in
#755,
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/render-config.html#render-config)).
* Add ``ProfileMapping`` for Vertica by @perttus in #540, #688 and #741,
as
([documentation](https://astronomer.github.io/astronomer-cosmos/profiles/VerticaUserPassword.html)).
* Add ``ProfileMapping`` for Snowflake encrypted private key path by
@ivanstillfront in #608, as ([documentation](
https://astronomer.github.io/astronomer-cosmos/profiles/SnowflakeEncryptedPrivateKeyFilePem.html)).
* Add support for Snowflake encrypted private key environment variable
by @DanMawdsleyBA in #649
* Add ``DbtDocsGCSOperator`` for uploading dbt docs to GCS by @jbandoro
in #616,
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/generating-docs.html#upload-to-gcs)).
* Add cosmos/propagate_logs Airflow config support for disabling log
propagation by @agreenburg in #648,
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/logging.html)).
* Add operator_args ``full_refresh`` as a templated field by @joppevos
in #623
* Expose environment variables and dbt variables in ``ProjectConfig`` by
@jbandoro in #735
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/project-config.html#project-config-example)).
* Support disabling event tracking when using Cosmos profile mapping by
@jbandoro in #768,
([documentation](https://astronomer.github.io/astronomer-cosmos/profiles/index.html#disabling-dbt-event-tracking)).

**Enhancements**

* Make Pydantic an optional dependency by @pixie79 in #736
* Create a symbolic link to ``dbt_packages`` when ``dbt_deps`` is False
when using ``LoadMode.DBT_LS`` by @DanMawdsleyBA in #730
* Add ``aws_session_token`` for Athena mapping by @benjamin-awd in #663
* Retrieve temporary credentials from ``conn_id`` for Athena by @octiva
in #758
* Extend ``DbtDocsLocalOperator`` with static flag by @joppevos  in #759

**Bug fixes**

* Remove Pydantic upper version restriction so Cosmos can be used with
Airflow 2.8 by @jlaneve in #772

**Others**

* Replace flake8 for Ruff by @joppevos in #743
* Reduce code complexity to 8 by @joppevos in #738
* Speed up integration tests by @jbandoro in #732
* Fix README quickstart link in by @RNHTTR in #776
* Add package location to work with hatchling 1.19.0 by @jbandoro in
#761
* Fix type check error in ``DbtKubernetesBaseOperator.build_env_args``
by @jbandoro in #766
* Improve ``DBT_MANIFEST`` documentation by @dwreeves in #757
* Update conflict matrix between Airflow and dbt versions by @tatiana in
#731 and #779
* pre-commit updates in #775, #770, #762
ykuc pushed a commit to ykuc/astronomer-cosmos that referenced this pull request Jan 11, 2024
@tatiana @YuriyK90
Release 1.3.0 (astronomer#781)
3aa9331 
**Features**

* Add new parsing method ``LoadMode.DBT_LS_FILE`` by @woogakoki in astronomer#733
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/parsing-methods.html#dbt-ls-file)).
* Add support to select using (some) graph operators when using
``LoadMode.CUSTOM`` and ``LoadMode.DBT_MANIFEST`` by @tatiana in astronomer#728
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/selecting-excluding.html#using-select-and-exclude))
* Add support for dbt ``selector`` arg for DAG parsing by @jbandoro in
astronomer#755,
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/render-config.html#render-config)).
* Add ``ProfileMapping`` for Vertica by @perttus in astronomer#540, astronomer#688 and astronomer#741,
as
([documentation](https://astronomer.github.io/astronomer-cosmos/profiles/VerticaUserPassword.html)).
* Add ``ProfileMapping`` for Snowflake encrypted private key path by
@ivanstillfront in astronomer#608, as ([documentation](
https://astronomer.github.io/astronomer-cosmos/profiles/SnowflakeEncryptedPrivateKeyFilePem.html)).
* Add support for Snowflake encrypted private key environment variable
by @DanMawdsleyBA in astronomer#649
* Add ``DbtDocsGCSOperator`` for uploading dbt docs to GCS by @jbandoro
in astronomer#616,
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/generating-docs.html#upload-to-gcs)).
* Add cosmos/propagate_logs Airflow config support for disabling log
propagation by @agreenburg in astronomer#648,
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/logging.html)).
* Add operator_args ``full_refresh`` as a templated field by @joppevos
in astronomer#623
* Expose environment variables and dbt variables in ``ProjectConfig`` by
@jbandoro in astronomer#735
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/project-config.html#project-config-example)).
* Support disabling event tracking when using Cosmos profile mapping by
@jbandoro in astronomer#768,
([documentation](https://astronomer.github.io/astronomer-cosmos/profiles/index.html#disabling-dbt-event-tracking)).

**Enhancements**

* Make Pydantic an optional dependency by @pixie79 in astronomer#736
* Create a symbolic link to ``dbt_packages`` when ``dbt_deps`` is False
when using ``LoadMode.DBT_LS`` by @DanMawdsleyBA in astronomer#730
* Add ``aws_session_token`` for Athena mapping by @benjamin-awd in astronomer#663
* Retrieve temporary credentials from ``conn_id`` for Athena by @octiva
in astronomer#758
* Extend ``DbtDocsLocalOperator`` with static flag by @joppevos  in astronomer#759

**Bug fixes**

* Remove Pydantic upper version restriction so Cosmos can be used with
Airflow 2.8 by @jlaneve in astronomer#772

**Others**

* Replace flake8 for Ruff by @joppevos in astronomer#743
* Reduce code complexity to 8 by @joppevos in astronomer#738
* Speed up integration tests by @jbandoro in astronomer#732
* Fix README quickstart link in by @RNHTTR in astronomer#776
* Add package location to work with hatchling 1.19.0 by @jbandoro in
astronomer#761
* Fix type check error in ``DbtKubernetesBaseOperator.build_env_args``
by @jbandoro in astronomer#766
* Improve ``DBT_MANIFEST`` documentation by @dwreeves in astronomer#757
* Update conflict matrix between Airflow and dbt versions by @tatiana in
astronomer#731 and astronomer#779
* pre-commit updates in astronomer#775, astronomer#770, astronomer#762
arojasb3 pushed a commit to arojasb3/astronomer-cosmos that referenced this pull request Jul 14, 2024
@perttus @arojasb3
Fix map Vertica airflow connection schema for vertica database (a…
11f8a1f 
…stronomer#741)

Map airflow connection schema for vertica database to keep it consistent
with other connection types and profiles.
Also Vertica Airflow provider assumes this:

https://github.com/apache/airflow/blob/395ac463494dba1478a05a32900218988495889c/airflow/providers/vertica/hooks/vertica.py#L72

Closes: astronomer#740 
Related: astronomer#538

Signed-off-by: Perttu Salonen <perttu.salonen@nitor.com>
arojasb3 pushed a commit to arojasb3/astronomer-cosmos that referenced this pull request Jul 14, 2024
@tatiana @arojasb3
Release 1.3.0 (astronomer#781)
0fe721e 
**Features**

* Add new parsing method ``LoadMode.DBT_LS_FILE`` by @woogakoki in astronomer#733
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/parsing-methods.html#dbt-ls-file)).
* Add support to select using (some) graph operators when using
``LoadMode.CUSTOM`` and ``LoadMode.DBT_MANIFEST`` by @tatiana in astronomer#728
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/selecting-excluding.html#using-select-and-exclude))
* Add support for dbt ``selector`` arg for DAG parsing by @jbandoro in
astronomer#755,
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/render-config.html#render-config)).
* Add ``ProfileMapping`` for Vertica by @perttus in astronomer#540, astronomer#688 and astronomer#741,
as
([documentation](https://astronomer.github.io/astronomer-cosmos/profiles/VerticaUserPassword.html)).
* Add ``ProfileMapping`` for Snowflake encrypted private key path by
@ivanstillfront in astronomer#608, as ([documentation](
https://astronomer.github.io/astronomer-cosmos/profiles/SnowflakeEncryptedPrivateKeyFilePem.html)).
* Add support for Snowflake encrypted private key environment variable
by @DanMawdsleyBA in astronomer#649
* Add ``DbtDocsGCSOperator`` for uploading dbt docs to GCS by @jbandoro
in astronomer#616,
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/generating-docs.html#upload-to-gcs)).
* Add cosmos/propagate_logs Airflow config support for disabling log
propagation by @agreenburg in astronomer#648,
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/logging.html)).
* Add operator_args ``full_refresh`` as a templated field by @joppevos
in astronomer#623
* Expose environment variables and dbt variables in ``ProjectConfig`` by
@jbandoro in astronomer#735
([documentation](https://astronomer.github.io/astronomer-cosmos/configuration/project-config.html#project-config-example)).
* Support disabling event tracking when using Cosmos profile mapping by
@jbandoro in astronomer#768,
([documentation](https://astronomer.github.io/astronomer-cosmos/profiles/index.html#disabling-dbt-event-tracking)).

**Enhancements**

* Make Pydantic an optional dependency by @pixie79 in astronomer#736
* Create a symbolic link to ``dbt_packages`` when ``dbt_deps`` is False
when using ``LoadMode.DBT_LS`` by @DanMawdsleyBA in astronomer#730
* Add ``aws_session_token`` for Athena mapping by @benjamin-awd in astronomer#663
* Retrieve temporary credentials from ``conn_id`` for Athena by @octiva
in astronomer#758
* Extend ``DbtDocsLocalOperator`` with static flag by @joppevos  in astronomer#759

**Bug fixes**

* Remove Pydantic upper version restriction so Cosmos can be used with
Airflow 2.8 by @jlaneve in astronomer#772

**Others**

* Replace flake8 for Ruff by @joppevos in astronomer#743
* Reduce code complexity to 8 by @joppevos in astronomer#738
* Speed up integration tests by @jbandoro in astronomer#732
* Fix README quickstart link in by @RNHTTR in astronomer#776
* Add package location to work with hatchling 1.19.0 by @jbandoro in
astronomer#761
* Fix type check error in ``DbtKubernetesBaseOperator.build_env_args``
by @jbandoro in astronomer#766
* Improve ``DBT_MANIFEST`` documentation by @dwreeves in astronomer#757
* Update conflict matrix between Airflow and dbt versions by @tatiana in
astronomer#731 and astronomer#779
* pre-commit updates in astronomer#775, astronomer#770, astronomer#762
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jbandoro jbandoro jbandoro left review comments

@tatiana tatiana tatiana approved these changes

Assignees

No one assigned

Labels

area:profile Related to ProfileConfig, like Athena, BigQuery, Clickhouse, Spark, Trino, etc lgtm This PR has been approved by a maintainer profile:vertica Related to Vertica ProfileConfig size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

1.3.0

Development

Successfully merging this pull request may close these issues.

FIX: map vertica airflow connection schema for vertica database

3 participants

@perttus @tatiana @jbandoro