You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/09/10 16:00:28 UTC
[GitHub] [airflow] SalAlba opened a new pull request #10864: Increase typing coverage for postgres provider #994
SalAlba opened a new pull request #10864:
URL: https://github.com/apache/airflow/pull/10864
This PR is about increasing typing coverage for the postgres provider. Part of: #9708
I change this file airflow/providers/postgres/hooks/postgres.py
closes: #9943
related: #9708
---
I go throw this guideline [Step 4: Prepare PR](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#step-4-prepare-pr)
## [Run the unit tests from the IDE or local virtualenv as you see fit.](https://github.com/apache/airflow/blob/master/TESTING.rst#running-unit-tests)
run the command bellow
``` bash
$ pytest tests/providers/postgres/
```
result
``` bash
============================================================ test session starts =============================================================
platform linux -- Python 3.7.0, pytest-6.0.1, py-1.9.0, pluggy-0.13.1 -- /home/salem/.pyenv/versions/3.7.0/envs/airflow-venv-370/bin/python3.7
cachedir: .pytest_cache
rootdir: /home/salem/WorkSpaces/python/airflow, configfile: pytest.ini
plugins: rerunfailures-9.0, forked-1.3.0, flaky-3.7.0, cov-2.10.1, requests-mock-1.8.0, timeouts-1.2.1, instafail-0.4.2, xdist-2.0.0
setup timeout: 0.0s, execution timeout: 0.0s, teardown timeout: 0.0s
collected 20 items
tests/providers/postgres/hooks/test_postgres.py::TestPostgresHookConn::test_get_conn PASSED [ 5%]
tests/providers/postgres/hooks/test_postgres.py::TestPostgresHookConn::test_get_conn_cursor PASSED [ 10%]
tests/providers/postgres/hooks/test_postgres.py::TestPostgresHookConn::test_get_conn_from_connection PASSED [ 15%]
tests/providers/postgres/hooks/test_postgres.py::TestPostgresHookConn::test_get_conn_from_connection_with_schema PASSED [ 20%]
tests/providers/postgres/hooks/test_postgres.py::TestPostgresHookConn::test_get_conn_non_default_id PASSED [ 25%]
tests/providers/postgres/hooks/test_postgres.py::TestPostgresHookConn::test_get_conn_rds_iam_postgres PASSED [ 30%]
tests/providers/postgres/hooks/test_postgres.py::TestPostgresHookConn::test_get_conn_rds_iam_redshift PASSED [ 35%]
tests/providers/postgres/hooks/test_postgres.py::TestPostgresHookConn::test_get_conn_with_invalid_cursor PASSED [ 40%]
tests/providers/postgres/hooks/test_postgres.py::TestPostgresHook::test_bulk_dump SKIPPED [ 45%]
tests/providers/postgres/hooks/test_postgres.py::TestPostgresHook::test_bulk_load SKIPPED [ 50%]
tests/providers/postgres/hooks/test_postgres.py::TestPostgresHook::test_copy_expert SKIPPED [ 55%]
tests/providers/postgres/hooks/test_postgres.py::TestPostgresHook::test_insert_rows SKIPPED [ 60%]
tests/providers/postgres/hooks/test_postgres.py::TestPostgresHook::test_insert_rows_replace SKIPPED [ 65%]
tests/providers/postgres/hooks/test_postgres.py::TestPostgresHook::test_insert_rows_replace_missing_replace_index_arg SKIPPED [ 70%]
tests/providers/postgres/hooks/test_postgres.py::TestPostgresHook::test_insert_rows_replace_missing_target_field_arg SKIPPED [ 75%]
tests/providers/postgres/hooks/test_postgres.py::TestPostgresHook::test_rowcount SKIPPED [ 80%]
tests/providers/postgres/operators/test_postgres.py::TestPostgres::test_overwrite_schema SKIPPED [ 85%]
tests/providers/postgres/operators/test_postgres.py::TestPostgres::test_postgres_operator_test SKIPPED [ 90%]
tests/providers/postgres/operators/test_postgres.py::TestPostgres::test_postgres_operator_test_multi SKIPPED [ 95%]
tests/providers/postgres/operators/test_postgres.py::TestPostgres::test_vacuum SKIPPED [100%]
============================================================== warnings summary ==============================================================
================================================== 8 passed, 12 skipped, 1 warning in 0.91s ==================================================
```
## [Run the tests in Breeze.](https://github.com/apache/airflow/blob/master/TESTING.rst#running-tests-for-a-specified-target-using-breeze-from-the-host)
run the command bellow
``` bash
$ sudo ./breeze tests tests/providers/postgres
```
results
``` bash
============================================================== warnings summary ==============================================================
ERROR tests/providers/postgres/hooks/test_postgres.py::TestPostgresHookConn::test_get_conn - Failed: Timeout >10.0s
ERROR tests/providers/postgres/hooks/test_postgres.py::TestPostgresHookConn::test_get_conn_cursor - Failed: Timeout >10.0s
ERROR tests/providers/postgres/hooks/test_postgres.py::TestPostgresHookConn::test_get_conn_from_connection - Failed: Timeout >10.0s
ERROR tests/providers/postgres/hooks/test_postgres.py::TestPostgresHookConn::test_get_conn_from_connection_with_schema - Failed: Timeout >1...
ERROR tests/providers/postgres/hooks/test_postgres.py::TestPostgresHookConn::test_get_conn_non_default_id - Failed: Timeout >10.0s
ERROR tests/providers/postgres/hooks/test_postgres.py::TestPostgresHookConn::test_get_conn_rds_iam_postgres - Failed: Timeout >10.0s
ERROR tests/providers/postgres/hooks/test_postgres.py::TestPostgresHookConn::test_get_conn_rds_iam_redshift - Failed: Timeout >10.0s
ERROR tests/providers/postgres/hooks/test_postgres.py::TestPostgresHookConn::test_get_conn_with_invalid_cursor - Failed: Timeout >10.0s
================================================= 12 skipped, 2 warnings, 8 errors in 15.06s =================================================
Regular tests. NOT Updating Quarantine Issue!
```
## [Running static checks](https://github.com/apache/airflow/blob/master/BREEZE.rst#running-static-checks)
I don't know at this time which static test should run for this Issue but I used the default one as shown below
``` bash
sudo ./breeze static-check mypy
```
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#discussion_r487877224
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -53,13 +58,13 @@ class PostgresHook(DbApiHook):
default_conn_name = 'postgres_default'
supports_autocommit = True
- def __init__(self, *args, **kwargs):
+ def __init__(self, *args, **kwargs) -> None:
super().__init__(*args, **kwargs)
self.schema = kwargs.pop("schema", None)
self.connection = kwargs.pop("connection", None)
self.conn = None
Review comment:
Yes π
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek edited a comment on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek edited a comment on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-692131377
Is your `master` same as `apache/master`? `git fetch` does not apply remote changes to your local branch. I would suggest to rebase onto `apache/master` and do this always to avoid stale information
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on a change in pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on a change in pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#discussion_r487218955
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
+Backups/
+Cache/
+CachedData/
+CachedExtensions/
+Code Cache/
+Cookies
+Cookies-journal
+GPUCache/
+Network Persistent State
+Session Storage/
+TransportSecurity
+User/
+languagepacks.json
+machineid
+rapid_render.json
+storage.json
+tmp.md
Review comment:
This is done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-693475734
Awesome work, congrats on your first merged pull request!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-691036418
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-693411666
Please rebase once this one is merged: https://github.com/apache/airflow/pull/10973
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-693356588
@SalAlba `pre-commit install` and this will run all check with every commit you do and only on changed files π
https://pre-commit.com/#3-install-the-git-hook-scripts
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-691222028
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#discussion_r487864037
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -53,13 +58,13 @@ class PostgresHook(DbApiHook):
default_conn_name = 'postgres_default'
supports_autocommit = True
- def __init__(self, *args, **kwargs):
+ def __init__(self, *args, **kwargs) -> None:
super().__init__(*args, **kwargs)
self.schema = kwargs.pop("schema", None)
self.connection = kwargs.pop("connection", None)
self.conn = None
Review comment:
Taking a look at the hook logic it seems that:
- `self.connection` is an Airflow `Connection` class so it provides information on what parameters should be used for establishing postgres connection
- `self.conn` is direct connection to postgres:
https://github.com/apache/airflow/blob/12a652f5344c7f03c3d780556ca1829b235fdb2d/airflow/providers/postgres/hooks/postgres.py#L104
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba edited a comment on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba edited a comment on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-692273340
Eureka guys !! now I start know how the stuffs should be done, and solve open source issues (before I didn't know since I'm new to this type of project)
For first time I should fork and clone the airflow repo, and make the configurations
```
git clone https://......
```
then just onec add ref to remote official airflow repo like bellow
```
git remote add apache https://github.com/apache/airflow.git
```
then fixing issues and make life happier :smile:, when it come to commit I should do this
```
git add .
git commit -m '.....'
```
then do the rebaseing like bellow, with `squash`
```
git fetch --all
git rebase apache/master
```
in case there are conflict fix it, if there are need of pullig also do and then
```
git push
```
After all of that should come finally the `PR` From MyRepo/master -->> apache/master
So my question now I'm right ? if yes should I close this current PR and open new fresh and clean `PR` for my actual issue (Increase typing coverage for postgres provider) ??
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on a change in pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on a change in pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#discussion_r487875791
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -53,13 +58,13 @@ class PostgresHook(DbApiHook):
default_conn_name = 'postgres_default'
supports_autocommit = True
- def __init__(self, *args, **kwargs):
+ def __init__(self, *args, **kwargs) -> None:
super().__init__(*args, **kwargs)
self.schema = kwargs.pop("schema", None)
self.connection = kwargs.pop("connection", None)
self.conn = None
Review comment:
This Airflow Connection come from this class I assume `from airflow.models import Connection`
so can we make this change :
```diff
from airflow.models import Connection
- self.connection = kwargs.pop("connection", None)
+ self.connection: Optional[Connection] = kwargs.pop("connection", None)
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek merged pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek merged pull request #10864:
URL: https://github.com/apache/airflow/pull/10864
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#discussion_r486604058
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
Review comment:
Can you say more about it?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-692567297
@SalAlba Do not worry. This is a community. We use the [logical time](https://en.wikipedia.org/wiki/Logical_clock) here, not real-time :-D It's essential to capture chronological and causal relationships, not a specific hour on a watch on your wrist.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on a change in pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on a change in pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#discussion_r486953545
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
Review comment:
Ok, I'm back guess, will the `tmp` mean temporary, but this changes should be just local on my machine :smile:
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -191,7 +203,7 @@ def get_iam_token(self, conn):
return login, token, port
@staticmethod
- def _generate_insert_sql(table, values, target_fields, replace, **kwargs):
+ def _generate_insert_sql(table: str, values: Tuple[object, ...], target_fields: Iterable[str], replace: bool, **kwargs) -> str:
Review comment:
@turbaszek I will fix that right now :+1:
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
Review comment:
Ok, I'm back guys, will the `tmp` mean temporary, but this changes should be just local on my machine :smile:
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
+Backups/
+Cache/
+CachedData/
+CachedExtensions/
+Code Cache/
+Cookies
+Cookies-journal
+GPUCache/
+Network Persistent State
+Session Storage/
+TransportSecurity
+User/
+languagepacks.json
+machineid
+rapid_render.json
+storage.json
+tmp.md
Review comment:
This is done
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
Review comment:
Ok, I'm back guess, will the `tmp` mean temporary, but this changes should be just local on my machine :smile:
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -191,7 +203,7 @@ def get_iam_token(self, conn):
return login, token, port
@staticmethod
- def _generate_insert_sql(table, values, target_fields, replace, **kwargs):
+ def _generate_insert_sql(table: str, values: Tuple[object, ...], target_fields: Iterable[str], replace: bool, **kwargs) -> str:
Review comment:
@turbaszek I will fix that right now :+1:
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
Review comment:
Ok, I'm back guys, will the `tmp` mean temporary, but this changes should be just local on my machine :smile:
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
+Backups/
+Cache/
+CachedData/
+CachedExtensions/
+Code Cache/
+Cookies
+Cookies-journal
+GPUCache/
+Network Persistent State
+Session Storage/
+TransportSecurity
+User/
+languagepacks.json
+machineid
+rapid_render.json
+storage.json
+tmp.md
Review comment:
This is done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-693404311
```
git ls-files | grep airflow/providers/google | xargs pre-commit run --files
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-692119459
I'm wondering If I did some bad steps :warning: , most of codes/commits come from other developers, and I suppose that happen when I do this steps :
```
git remote add apache https://github.com/apache/airflow.git
git fetch --all
git rebase apache/master
git push
```
then I make some changes in my code especially on this file just `airflow/providers/postgres/hooks/postgres.py
` and then I checkout to my branch because, I wanted to run all actions (the whole tests ) in PR #10864 , so in command looks like this.
```
git checkout sal-dev
git rebase master
git push -u origin sal-dev
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek edited a comment on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek edited a comment on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-692535450
> So my question now I'm right ? if yes should I close this current PR and open new fresh and clean `PR` for my actual issue (Increase typing coverage for postgres provider) ??
Assuming your changes should affect only `airflow/airflow/providers/postgres/hooks/postgres.py` I would the following:
- copy this file to other location outside repository
- do `git fetch --all` and `git reset --hard apache/master` on your branch `sal-dev`
- copy back the file
- create commit
- `git push --force-with-lease`
I think this is the simplest way to clean up this PR. And here are some useful tips:
https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#id49
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on a change in pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on a change in pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#discussion_r487848331
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -53,13 +58,13 @@ class PostgresHook(DbApiHook):
default_conn_name = 'postgres_default'
supports_autocommit = True
- def __init__(self, *args, **kwargs):
+ def __init__(self, *args, **kwargs) -> None:
super().__init__(*args, **kwargs)
self.schema = kwargs.pop("schema", None)
self.connection = kwargs.pop("connection", None)
self.conn = None
Review comment:
Hi, could someone tell me please, what's the diff between this tow line of code (two connections)
are both same connection ? I assume no
``` python
self.connection = kwargs.pop("connection", None)
self.conn = None
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-692919402
> Please rebase to latest master - we had some breaking change a commit ago and the PR will need to be rebased.
@potiuk I will do that according to this tips [How to rebase PR](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#id9)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-691222028
The CI errors seem to be similar to #10153 as reported by @yuqian90
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-692273340
Eureka guys !! now I start know how the stuffs should be done, and solve open source issues (before I didn't know since I'm new to this type of project)
For first time I should fork and clone the airflow repo, and make the configurations
```
git clone https://......
```
then just onec add ref to remote official airflow repo like bellow
```
git remote add apache https://github.com/apache/airflow.git
```
then fixing issues and make life happier :smile:, when it come to commit I should do this
```
git add .
git commit -m '.....'
```
then do the rebaseing like bellow
```
git fetch --all
git rebase apache/master
```
in case there are conflict fix it, if there are need of pullig also do and then
```
git push
```
After all of that should come finally the `PR` From MyRepo/master -->> apache/master
So my question now I'm right ? if yes should I close this current PR and open new fresh and clean `PR` for my actual issue (Increase typing coverage for postgres provider) ??
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-693342672
Hi guys @mik-laj, @turbaszek, @potiuk,
could some help I'm I want to run ` pre-commits` to run some test, at the moment I using this one
```
pre-commit run mypy --all-files
```
but it take a lot time on my computer, so my question how can I use it for specific sub folder or file right now I'm using in this way
```
pre-commit run mypy --files airflow/providers/postgres/*
pre-commit run mypy --files airflow/providers/postgres/hooks/postgres.py
```
but I'm not sure if this is the best way, or if there are another possible and better ways, so any ideas or tips ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-691222028
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#discussion_r487189853
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
+Backups/
+Cache/
+CachedData/
+CachedExtensions/
+Code Cache/
+Cookies
+Cookies-journal
+GPUCache/
+Network Persistent State
+Session Storage/
+TransportSecurity
+User/
+languagepacks.json
+machineid
+rapid_render.json
+storage.json
+tmp.md
Review comment:
@SalAlba can you please remove this change? It's unrelated to the PR :)
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
+Backups/
+Cache/
+CachedData/
+CachedExtensions/
+Code Cache/
+Cookies
+Cookies-journal
+GPUCache/
+Network Persistent State
+Session Storage/
+TransportSecurity
+User/
+languagepacks.json
+machineid
+rapid_render.json
+storage.json
+tmp.md
Review comment:
@SalAlba can you please remove this change? It's unrelated to the PR :)
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
+Backups/
+Cache/
+CachedData/
+CachedExtensions/
+Code Cache/
+Cookies
+Cookies-journal
+GPUCache/
+Network Persistent State
+Session Storage/
+TransportSecurity
+User/
+languagepacks.json
+machineid
+rapid_render.json
+storage.json
+tmp.md
Review comment:
@SalAlba can you please remove this change? It's unrelated to the PR :)
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
+Backups/
+Cache/
+CachedData/
+CachedExtensions/
+Code Cache/
+Cookies
+Cookies-journal
+GPUCache/
+Network Persistent State
+Session Storage/
+TransportSecurity
+User/
+languagepacks.json
+machineid
+rapid_render.json
+storage.json
+tmp.md
Review comment:
@SalAlba can you please remove this change? It's unrelated to the PR :)
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
+Backups/
+Cache/
+CachedData/
+CachedExtensions/
+Code Cache/
+Cookies
+Cookies-journal
+GPUCache/
+Network Persistent State
+Session Storage/
+TransportSecurity
+User/
+languagepacks.json
+machineid
+rapid_render.json
+storage.json
+tmp.md
Review comment:
@SalAlba can you please remove this change? It's unrelated to the PR :)
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
+Backups/
+Cache/
+CachedData/
+CachedExtensions/
+Code Cache/
+Cookies
+Cookies-journal
+GPUCache/
+Network Persistent State
+Session Storage/
+TransportSecurity
+User/
+languagepacks.json
+machineid
+rapid_render.json
+storage.json
+tmp.md
Review comment:
@SalAlba can you please remove this change? It's unrelated to the PR :)
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
+Backups/
+Cache/
+CachedData/
+CachedExtensions/
+Code Cache/
+Cookies
+Cookies-journal
+GPUCache/
+Network Persistent State
+Session Storage/
+TransportSecurity
+User/
+languagepacks.json
+machineid
+rapid_render.json
+storage.json
+tmp.md
Review comment:
@SalAlba can you please remove this change? It's unrelated to the PR :)
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
+Backups/
+Cache/
+CachedData/
+CachedExtensions/
+Code Cache/
+Cookies
+Cookies-journal
+GPUCache/
+Network Persistent State
+Session Storage/
+TransportSecurity
+User/
+languagepacks.json
+machineid
+rapid_render.json
+storage.json
+tmp.md
Review comment:
@SalAlba can you please remove this change? It's unrelated to the PR :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-691036418
``` bash
MP_DIR=$(mktemp -d); mypy --linecount-report ${MP_DIR} airflow/; cat "${MP_DIR}/linecount.txt" | grep providers | grep -v example_dags | awk '
$4 != 0 {
split($5, a, ".");
print 100.00 * ($3/$4), a[3]
}'| awk '
{ sum[$2] += $1; N[$2]++ }
END {
for (key in sum) {
avg = sum[key] / N[key];
printf "%f %s\n", avg, key;
}
}' | sort -g
```
```
0,000000 dingding
0,000000 opsgenie
0,000000 presto
0,000000 qubole
0,000000 samba
0,000000 zendesk
16,666650 ssh
16,666667 oracle
16,666667 yandex
16,666700 odbc
21,428550 databricks
25,000000 exasol
25,000000 jdbc
25,000000 singularity
25,000000 snowflake
27,380950 slack
27,964227 amazon
33,333333 sftp
46,153850 imap
50,733332 microsoft
66,761375 cncf
67,857150 salesforce
69,117976 google
71,250000 ftp
75,000000 postgres <<< ==== was 25.00 now 75.00 but should be 100
83,333300 openfaas
87,500000 redis
87,921875 apache
88,333340 mysql
88,888900 papermill
96,875000 mongo
100,000000 celery
100,000000 cloudant
100,000000 datadog
100,000000 discord
100,000000 docker
100,000000 elasticsearch
100,000000 facebook
100,000000 grpc
100,000000 hashicorp
100,000000 http
100,000000 jenkins
100,000000 jira
100,000000 pagerduty
100,000000 segment
100,000000 sendgrid
100,000000 sqlite
100,000000 vertica
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#discussion_r487541603
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -53,13 +58,13 @@ class PostgresHook(DbApiHook):
default_conn_name = 'postgres_default'
supports_autocommit = True
- def __init__(self, *args, **kwargs):
+ def __init__(self, *args, **kwargs) -> None:
super().__init__(*args, **kwargs)
self.schema = kwargs.pop("schema", None)
self.connection = kwargs.pop("connection", None)
self.conn = None
Review comment:
```suggestion
self.conn: Optional[Any] = None
```
Should solve the issue from CI. I'm not exactly sure what `self.conn` will become but it should be some object according to this line:
https://github.com/apache/airflow/blob/12a652f5344c7f03c3d780556ca1829b235fdb2d/airflow/providers/postgres/hooks/postgres.py#L104
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-691254882
> > > The CI errors seem to be similar to #10153 as reported by @yuqian90
> >
> >
> > Seems to be this issue: #10877 @SalAlba can you rebase please?
>
> **For rebase** I made this
>
> ```
> git checkout sal-dev
>
> git rebase master
>
> git checkout master
>
> git merge sal-dev
> ```
>
> _Hopefully this is the correct way !_
This is nearly perfect. You should first fetch newest changes from apache/master. If you don't have apache remote I suggest this:
```
git remote add apache git@github.com:apache/airflow.git
git fetch --all
# Then on your branch
git rebase apache/master
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-692591050
I feel something mysterious hiding in this logical time :thinking:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-692542764
> @SalAlba I fixed your branch. Please hard reset your branch to the positions of remote branch. (`#07764168da7d356b402c2662f5b8cdb4810bd171`)
>
> ```
> git checkout sal-dev
> git fetch --all
> git reset --hard origin/sal-dev # or git reset --hard 07764168da7d356b402c2662f5b8cdb4810bd171
> ```
I will do that just give me 1 hour :disappointed: I have to finish one thing at this moment
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#discussion_r487541603
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -53,13 +58,13 @@ class PostgresHook(DbApiHook):
default_conn_name = 'postgres_default'
supports_autocommit = True
- def __init__(self, *args, **kwargs):
+ def __init__(self, *args, **kwargs) -> None:
super().__init__(*args, **kwargs)
self.schema = kwargs.pop("schema", None)
self.connection = kwargs.pop("connection", None)
self.conn = None
Review comment:
```suggestion
self.conn: Optional[Any] = None
```
Should solve the issue from CI. I'm not exactly sure what `self.conn` will became but it should be some object according to this line:
https://github.com/apache/airflow/blob/12a652f5344c7f03c3d780556ca1829b235fdb2d/airflow/providers/postgres/hooks/postgres.py#L104
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on a change in pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on a change in pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#discussion_r486953545
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
Review comment:
Ok, I'm back guess, will the `tmp` mean temporary, but this changes should be just local on my machine :smile:
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -191,7 +203,7 @@ def get_iam_token(self, conn):
return login, token, port
@staticmethod
- def _generate_insert_sql(table, values, target_fields, replace, **kwargs):
+ def _generate_insert_sql(table: str, values: Tuple[object, ...], target_fields: Iterable[str], replace: bool, **kwargs) -> str:
Review comment:
@turbaszek I will fix that right now :+1:
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
Review comment:
Ok, I'm back guys, will the `tmp` mean temporary, but this changes should be just local on my machine :smile:
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
+Backups/
+Cache/
+CachedData/
+CachedExtensions/
+Code Cache/
+Cookies
+Cookies-journal
+GPUCache/
+Network Persistent State
+Session Storage/
+TransportSecurity
+User/
+languagepacks.json
+machineid
+rapid_render.json
+storage.json
+tmp.md
Review comment:
This is done
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
Review comment:
Ok, I'm back guess, will the `tmp` mean temporary, but this changes should be just local on my machine :smile:
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -191,7 +203,7 @@ def get_iam_token(self, conn):
return login, token, port
@staticmethod
- def _generate_insert_sql(table, values, target_fields, replace, **kwargs):
+ def _generate_insert_sql(table: str, values: Tuple[object, ...], target_fields: Iterable[str], replace: bool, **kwargs) -> str:
Review comment:
@turbaszek I will fix that right now :+1:
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
Review comment:
Ok, I'm back guys, will the `tmp` mean temporary, but this changes should be just local on my machine :smile:
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
+Backups/
+Cache/
+CachedData/
+CachedExtensions/
+Code Cache/
+Cookies
+Cookies-journal
+GPUCache/
+Network Persistent State
+Session Storage/
+TransportSecurity
+User/
+languagepacks.json
+machineid
+rapid_render.json
+storage.json
+tmp.md
Review comment:
This is done
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
Review comment:
Ok, I'm back guess, will the `tmp` mean temporary, but this changes should be just local on my machine :smile:
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -191,7 +203,7 @@ def get_iam_token(self, conn):
return login, token, port
@staticmethod
- def _generate_insert_sql(table, values, target_fields, replace, **kwargs):
+ def _generate_insert_sql(table: str, values: Tuple[object, ...], target_fields: Iterable[str], replace: bool, **kwargs) -> str:
Review comment:
@turbaszek I will fix that right now :+1:
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
Review comment:
Ok, I'm back guys, will the `tmp` mean temporary, but this changes should be just local on my machine :smile:
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
+Backups/
+Cache/
+CachedData/
+CachedExtensions/
+Code Cache/
+Cookies
+Cookies-journal
+GPUCache/
+Network Persistent State
+Session Storage/
+TransportSecurity
+User/
+languagepacks.json
+machineid
+rapid_render.json
+storage.json
+tmp.md
Review comment:
This is done
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
Review comment:
Ok, I'm back guess, will the `tmp` mean temporary, but this changes should be just local on my machine :smile:
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -191,7 +203,7 @@ def get_iam_token(self, conn):
return login, token, port
@staticmethod
- def _generate_insert_sql(table, values, target_fields, replace, **kwargs):
+ def _generate_insert_sql(table: str, values: Tuple[object, ...], target_fields: Iterable[str], replace: bool, **kwargs) -> str:
Review comment:
@turbaszek I will fix that right now :+1:
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
Review comment:
Ok, I'm back guys, will the `tmp` mean temporary, but this changes should be just local on my machine :smile:
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
+Backups/
+Cache/
+CachedData/
+CachedExtensions/
+Code Cache/
+Cookies
+Cookies-journal
+GPUCache/
+Network Persistent State
+Session Storage/
+TransportSecurity
+User/
+languagepacks.json
+machineid
+rapid_render.json
+storage.json
+tmp.md
Review comment:
This is done
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
Review comment:
Ok, I'm back guess, will the `tmp` mean temporary, but this changes should be just local on my machine :smile:
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -191,7 +203,7 @@ def get_iam_token(self, conn):
return login, token, port
@staticmethod
- def _generate_insert_sql(table, values, target_fields, replace, **kwargs):
+ def _generate_insert_sql(table: str, values: Tuple[object, ...], target_fields: Iterable[str], replace: bool, **kwargs) -> str:
Review comment:
@turbaszek I will fix that right now :+1:
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
Review comment:
Ok, I'm back guys, will the `tmp` mean temporary, but this changes should be just local on my machine :smile:
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
+Backups/
+Cache/
+CachedData/
+CachedExtensions/
+Code Cache/
+Cookies
+Cookies-journal
+GPUCache/
+Network Persistent State
+Session Storage/
+TransportSecurity
+User/
+languagepacks.json
+machineid
+rapid_render.json
+storage.json
+tmp.md
Review comment:
This is done
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
Review comment:
Ok, I'm back guess, will the `tmp` mean temporary, but this changes should be just local on my machine :smile:
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -191,7 +203,7 @@ def get_iam_token(self, conn):
return login, token, port
@staticmethod
- def _generate_insert_sql(table, values, target_fields, replace, **kwargs):
+ def _generate_insert_sql(table: str, values: Tuple[object, ...], target_fields: Iterable[str], replace: bool, **kwargs) -> str:
Review comment:
@turbaszek I will fix that right now :+1:
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
Review comment:
Ok, I'm back guys, will the `tmp` mean temporary, but this changes should be just local on my machine :smile:
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
+Backups/
+Cache/
+CachedData/
+CachedExtensions/
+Code Cache/
+Cookies
+Cookies-journal
+GPUCache/
+Network Persistent State
+Session Storage/
+TransportSecurity
+User/
+languagepacks.json
+machineid
+rapid_render.json
+storage.json
+tmp.md
Review comment:
This is done
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
Review comment:
Ok, I'm back guess, will the `tmp` mean temporary, but this changes should be just local on my machine :smile:
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -191,7 +203,7 @@ def get_iam_token(self, conn):
return login, token, port
@staticmethod
- def _generate_insert_sql(table, values, target_fields, replace, **kwargs):
+ def _generate_insert_sql(table: str, values: Tuple[object, ...], target_fields: Iterable[str], replace: bool, **kwargs) -> str:
Review comment:
@turbaszek I will fix that right now :+1:
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
Review comment:
Ok, I'm back guys, will the `tmp` mean temporary, but this changes should be just local on my machine :smile:
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
+Backups/
+Cache/
+CachedData/
+CachedExtensions/
+Code Cache/
+Cookies
+Cookies-journal
+GPUCache/
+Network Persistent State
+Session Storage/
+TransportSecurity
+User/
+languagepacks.json
+machineid
+rapid_render.json
+storage.json
+tmp.md
Review comment:
This is done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-693486993
> Awesome work, congrats on your first merged pull request!
Thank you airflow bot :joy:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-692535450
> So my question now I'm right ? if yes should I close this current PR and open new fresh and clean `PR` for my actual issue (Increase typing coverage for postgres provider) ??
Assuming your changes should affect only `airflow/airflow/providers/postgres/hooks/postgres.py` I would the following:
- copy this file to other location outside repository
- do `git reset --hard apache/master` on your branch `sal-dev`
- copy back the file
- create commit
- `git push --force-with-lease`
I think this is the simplest way to clean up this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-692541795
@SalAlba I fixed your branch. Please hard reset your branch to the positions of remote branch. (``#07764168da7d356b402c2662f5b8cdb4810bd171``)
```
git checkout sal-dev
git fetch --all
git reset --hard origin/sal-dev # or git reset --hard 07764168da7d356b402c2662f5b8cdb4810bd171
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-693399453
> @SalAlba `pre-commit install` and this will run all check with every commit you do and only on changed files
> https://pre-commit.com/#3-install-the-git-hook-scripts
thanks @turbaszek this one I know :smile: more I'm asking how run this test for one folder, but I think I fix the problem so thanks again
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba edited a comment on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba edited a comment on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-693413185
aaa ok get it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on a change in pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on a change in pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#discussion_r486953545
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
Review comment:
Ok, I'm back guys, will the `tmp` mean temporary, but this changes should be just local on my machine :smile:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on pull request #10864: Increase typing coverage for postgres provider #994
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-690401392
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst)
Here are some useful points:
- Pay attention to the quality of your code (flake8, pylint and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/master/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/master/docs/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/master/BREEZE.rst) for testing locally, itβs a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#coding-style-and-best-practices).
Apache Airflow is a community-driven project and together we are making it better π.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://apache-airflow-slack.herokuapp.com/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#discussion_r487189853
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
+Backups/
+Cache/
+CachedData/
+CachedExtensions/
+Code Cache/
+Cookies
+Cookies-journal
+GPUCache/
+Network Persistent State
+Session Storage/
+TransportSecurity
+User/
+languagepacks.json
+machineid
+rapid_render.json
+storage.json
+tmp.md
Review comment:
@SalAlba can you please remove this change? It's unrelated to the PR :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-691036418
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-693409662
What is the No-tabs checker in static test [TETS FAIL DETAIL](https://github.com/apache/airflow/pull/10864/checks?check_run_id=1123251919)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on a change in pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on a change in pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#discussion_r488781622
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -157,7 +165,7 @@ def _serialize_cell(cell, conn):
"""
return cell
- def get_iam_token(self, conn):
+ def get_iam_token(self, conn: connection) -> Tuple[str, str, int]:
Review comment:
the arg `conn` of method `get_iam_token(...)`, is airflow connection or the direct connection to PostgreSQL ?
1. like this `self.connection` is an Airflow Connection class so it provides information on what parameters should be used for establishing PostgreSQL connection.
2. like this `self.conn` is direct connection to PostgreSQL.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#discussion_r486611993
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
Review comment:
I think it may be unrelated
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-692849259
Please rebase to latest master - we had some breaking change a commit ago and the PR will need to be rebased.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#discussion_r486614542
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -191,7 +203,7 @@ def get_iam_token(self, conn):
return login, token, port
@staticmethod
- def _generate_insert_sql(table, values, target_fields, replace, **kwargs):
+ def _generate_insert_sql(table: str, values: Tuple[object, ...], target_fields: Iterable[str], replace: bool, **kwargs) -> str:
Review comment:
```suggestion
def _generate_insert_sql(table: str, values: Tuple[str, ...], target_fields: Iterable[str], replace: bool, **kwargs) -> str:
```
According to `DbApiHook._serialize_cell`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba edited a comment on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba edited a comment on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-693409662
What is the No-tabs checker in static test [TETS FAIL DETAIL](https://github.com/apache/airflow/pull/10864/checks?check_run_id=1123251919)
tabs are this `'\t'` or something else ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#discussion_r487189853
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
+Backups/
+Cache/
+CachedData/
+CachedExtensions/
+Code Cache/
+Cookies
+Cookies-journal
+GPUCache/
+Network Persistent State
+Session Storage/
+TransportSecurity
+User/
+languagepacks.json
+machineid
+rapid_render.json
+storage.json
+tmp.md
Review comment:
@SalAlba can you please remove this change? It's unrelated to the PR :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek edited a comment on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek edited a comment on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-692535450
> So my question now I'm right ? if yes should I close this current PR and open new fresh and clean `PR` for my actual issue (Increase typing coverage for postgres provider) ??
Assuming your changes should affect only `airflow/airflow/providers/postgres/hooks/postgres.py` I would the following:
- copy this file to other location outside repository
- do `git fetch --all` and `git reset --hard apache/master` on your branch `sal-dev`
- copy back the file
- create commit
- `git push --force-with-lease`
I think this is the simplest way to clean up this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-691231031
> The CI errors seem to be similar to #10153 as reported by @yuqian90
Seems to be this issue: https://github.com/apache/airflow/pull/10877 @SalAlba can you rebase please?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-692131377
Is your `master` same as `apache/master`? `git fetch` does not apply remote changes to your local branch
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-691250121
> > The CI errors seem to be similar to #10153 as reported by @yuqian90
>
> Seems to be this issue: #10877 @SalAlba can you rebase please?
**For rebase** I made this
```
git checkout sal-dev
git rebase master
git checkout master
git merge sal-dev
```
_Hopefully this is the correct way !_
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-693413185
so where to rebase ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on a change in pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on a change in pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#discussion_r486953545
##########
File path: .gitignore
##########
@@ -205,3 +205,22 @@ Chart.lock
# Might be generated when you build wheels
pip-wheel-metadata
+
+# tmp ...
Review comment:
Ok, I'm back guess, will the `tmp` mean temporary, but this changes should be just local on my machine :smile:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-691090705
I run the command bellow according to this note, so my file `providers/postgres` is reformatted
```
Make sure your code fulfils all the static code checks we have in our code. The easiest way to make sure of that is to use pre-commit hooks
```
``` bash
$ sudo pre-commit run --all-files
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-692272078
> stale
> Is your `master` same as `apache/master`? `git fetch` does not apply remote changes to your local branch. I would suggest to rebase onto `apache/master` and do this always to avoid stale information
my master yes are the same the only diff in file `airflow/providers/postgres/hooks/postgres.py`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-692589712
> @SalAlba Do not worry. This is a community. We use the [logical time](https://en.wikipedia.org/wiki/Logical_clock) here, not real-time :-D It's essential to capture chronological and causal relationships, not a specific hour on a watch on your wrist.
@mik-laj is right, and he's an expert in that π
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-691036418
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-693411976
That Pr is now merged.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#issuecomment-691325848
ok git rebase done, time now for skipping ` CI errors ` :fire:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SalAlba commented on a change in pull request #10864: Increase typing coverage for postgres provider
Posted by GitBox <gi...@apache.org>.
SalAlba commented on a change in pull request #10864:
URL: https://github.com/apache/airflow/pull/10864#discussion_r486956770
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -191,7 +203,7 @@ def get_iam_token(self, conn):
return login, token, port
@staticmethod
- def _generate_insert_sql(table, values, target_fields, replace, **kwargs):
+ def _generate_insert_sql(table: str, values: Tuple[object, ...], target_fields: Iterable[str], replace: bool, **kwargs) -> str:
Review comment:
@turbaszek I will fix that right now :+1:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org