Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to Python 3.9 #11763

Merged
merged 13 commits into from
Apr 12, 2022
Merged

Upgrade to Python 3.9 #11763

merged 13 commits into from
Apr 12, 2022

Conversation

bleonard
Copy link
Contributor

@bleonard bleonard commented Apr 6, 2022

What

Investigating #11657 to upgrade Python to 3.9

How

Change all the 3.7 to 3.9 and run the tests

🚨 User Impact 🚨

There may be some python connectors with issues that we'll need to update.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

@github-actions github-actions bot added area/connectors Connector related issues area/documentation Improvements or additions to documentation CDK Connector Development Kit labels Apr 6, 2022
@bleonard bleonard temporarily deployed to more-secrets April 6, 2022 17:35 Inactive
@bleonard bleonard temporarily deployed to more-secrets April 6, 2022 17:35 Inactive
@bleonard
Copy link
Contributor Author

bleonard commented Apr 6, 2022

As a start, I ran all the ./gradlew :airbyte-integrations:connectors:*:unitTest tasks

Here are the ones that failed:

./gradlew :airbyte-integrations:connectors:source-appstore-singer:unitTest
./gradlew :airbyte-integrations:connectors:source-zoom-singer:unitTest
./gradlew :airbyte-integrations:connectors:source-tiktok-marketing:unitTest
./gradlew :airbyte-integrations:connectors:source-yahoo-finance-price:unitTest
./gradlew :airbyte-integrations:connectors:destination-aws-datalake:unitTest
./gradlew :airbyte-integrations:connectors:source-google-ads:unitTest
./gradlew :airbyte-integrations:connectors:source-monday:unitTest
./gradlew :airbyte-integrations:connectors:source-dixa:unitTest
./gradlew :airbyte-integrations:connectors:source-snapchat-marketing:unitTest
./gradlew :airbyte-integrations:connectors:source-appsflyer-singer:unitTest
./gradlew :airbyte-integrations:connectors:source-s3:unitTest
./gradlew :airbyte-integrations:connectors:source-file-secure:unitTest

unit_test_failures.log

Summary:

source-appstore-singer: can't install cryptography

build/temp.macosx-12.3-arm64-3.9/_openssl.c:575:10: fatal error: 'openssl/opensslv.h' file not found
#include <openssl/opensslv.h>

source-zoom-singer: version mismatches

 The conflict is caused by:
     base-python 0.0.0 depends on jsonschema==3.2.0
     singer-python 5.9.0 depends on jsonschema==2.6.0

source-tiktok-marketing: expects secrets file

FileNotFoundError: [Errno 2] No such file or directory: 'secrets/prod_config.json'

source-yahoo-finance-price: tests failing

(False, 'No valid tickers provided')

destination-aws-datalake: no unit test

Task 'unitTest' not found in project ':airbyte-integrations:connectors:destination-aws-datalake'.

source-google-ads: expects secrets file

FileNotFoundError: [Errno 2] No such file or directory: 'secrets/config.json'

source-monday: expects secrets file

FileNotFoundError: [Errno 2] No such file or directory: 'secrets/config.json'

source-dixa: timing differences in unit test

Full diff:
  [
-  {'updated_after': 1625097600000, 'updated_before': 1625184000000},
?                        ^^^^                             ^^^
+  {'updated_after': 1625122800000, 'updated_before': 1625209200000},
?                        ^^^^                             ^ ++
-  {'updated_after': 1625184000000, 'updated_before': 1625259600000},
?                        ^^^
+  {'updated_after': 1625209200000, 'updated_before': 1625259600000},
?                        ^ ++
  ]

source-snapchat-marketing: expects secrets file

FileNotFoundError: [Errno 2] No such file or directory: 'secrets/config.json'

source-appsflyer-singer: version conflict

base-python 0.0.0 depends on requests
tap-appsflyer 0.0.12 depends on requests==2.25

source-s3: install issue

ERROR: Could not find a version that satisfies the requirement pyarrow==4.0.1 (from source-s3) (from versions: 0.9.0, 0.10.0, 0.11.0, 0.11.1, 0.12.0, 0.12.1, 0.13.0, 0.14.0, 0.15.1, 0.16.0, 0.17.0, 0.17.1, 1.0.0, 1.0.1, 2.0.0, 3.0.0, 4.0.0, 4.0.1, 5.0.0, 6.0.0, 6.0.1, 7.0.0)
ERROR: No matching distribution found for pyarrow==4.0.1

source-file-secure: install issue

ERROR: Could not find a version that satisfies the requirement pyarrow==3.0.0 (from source-file) (from versions: 0.9.0, 0.10.0, 0.11.0, 0.11.1, 0.12.0, 0.12.1, 0.13.0, 0.14.0, 0.15.1, 0.16.0, 0.17.0, 0.17.1, 1.0.0, 1.0.1, 2.0.0, 3.0.0, 4.0.0, 4.0.1, 5.0.0, 6.0.0, 6.0.1, 7.0.0)
ERROR: No matching distribution found for pyarrow==3.0.0

@davinchia
Copy link
Contributor

davinchia commented Apr 7, 2022

/test connector=source-airtable

🕑 source-airtable https://github.com/airbytehq/airbyte/actions/runs/2107995904
❌ source-airtable https://github.com/airbytehq/airbyte/actions/runs/2107995904
🐛

@bleonard
Copy link
Contributor Author

bleonard commented Apr 7, 2022

/test connector=source-airtable

🕑 source-airtable https://github.com/airbytehq/airbyte/actions/runs/2110220951
❌ source-airtable https://github.com/airbytehq/airbyte/actions/runs/2110220951
🐛

@bleonard bleonard temporarily deployed to more-secrets April 7, 2022 16:21 Inactive
@bleonard bleonard temporarily deployed to more-secrets April 7, 2022 16:21 Inactive
@bleonard bleonard temporarily deployed to more-secrets April 7, 2022 18:28 Inactive
@bleonard bleonard temporarily deployed to more-secrets April 7, 2022 18:28 Inactive
@bleonard
Copy link
Contributor Author

bleonard commented Apr 7, 2022

/test connector=source-airtable

🕑 source-airtable https://github.com/airbytehq/airbyte/actions/runs/2110858814
❌ source-airtable https://github.com/airbytehq/airbyte/actions/runs/2110858814
🐛

@bleonard bleonard temporarily deployed to more-secrets April 7, 2022 18:52 Inactive
@bleonard
Copy link
Contributor Author

bleonard commented Apr 7, 2022

/test connector=source-asana ref=bl-python-39

🕑 source-asana https://github.com/airbytehq/airbyte/actions/runs/2110983887
❌ source-asana https://github.com/airbytehq/airbyte/actions/runs/2110983887
🐛 https://gradle.com/s/kfizhoiffjif2
Python short test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestBasicRead::test_read[inputs0] - docker.errors.Contai...
FAILED test_full_refresh.py::TestFullRefresh::test_sequential_reads[inputs0]
SKIPPED [1] ../usr/local/lib/python3.7/site-packages/source_acceptance_test/plugin.py:56: Skipping TestIncremental.test_two_sequential_reads because not found in the config
=================== 2 failed, 18 passed, 1 skipped in 9.48s ====================

@davinchia davinchia temporarily deployed to more-secrets April 7, 2022 19:05 Inactive
@bleonard
Copy link
Contributor Author

bleonard commented Apr 7, 2022

/test connector=all ref=bl-python-39

🕑 all https://github.com/airbytehq/airbyte/actions/runs/2111051980
❌ all https://github.com/airbytehq/airbyte/actions/runs/2111051980
🐛

@davinchia davinchia temporarily deployed to more-secrets April 7, 2022 19:19 Inactive
@bleonard
Copy link
Contributor Author

bleonard commented Apr 7, 2022

/test connector=all ref=bl-python-39

🕑 all https://github.com/airbytehq/airbyte/actions/runs/2111141995
❌ all https://github.com/airbytehq/airbyte/actions/runs/2111141995
🐛

@davinchia davinchia temporarily deployed to more-secrets April 7, 2022 19:38 Inactive
@bleonard bleonard marked this pull request as ready for review April 8, 2022 00:09
@bleonard
Copy link
Contributor Author

bleonard commented Apr 8, 2022

/test connector=source-airtable ref=bl-python-39

🕑 source-airtable https://github.com/airbytehq/airbyte/actions/runs/2112263974
✅ source-airtable https://github.com/airbytehq/airbyte/actions/runs/2112263974
Python tests coverage:

Name                                                 Stmts   Miss  Cover
------------------------------------------------------------------------
source_acceptance_test/utils/__init__.py                 6      0   100%
source_acceptance_test/tests/__init__.py                 4      0   100%
source_acceptance_test/__init__.py                       2      0   100%
source_acceptance_test/tests/test_full_refresh.py       52      2    96%
source_acceptance_test/utils/asserts.py                 37      2    95%
source_acceptance_test/config.py                        74      6    92%
source_acceptance_test/utils/json_schema_helper.py     105     13    88%
source_acceptance_test/utils/common.py                  70     17    76%
source_acceptance_test/utils/compare.py                 62     23    63%
source_acceptance_test/tests/test_core.py              275    106    61%
source_acceptance_test/base.py                          10      4    60%
source_acceptance_test/utils/connector_runner.py       110     48    56%
source_acceptance_test/tests/test_incremental.py        69     38    45%
------------------------------------------------------------------------
TOTAL                                                  876    259    70%
Name                                                                                                                            Stmts   Miss  Cover
---------------------------------------------------------------------------------------------------------------------------------------------------
base_python/cdk/utils/casing.py                                                                                                     4      0   100%
base_python/__init__.py                                                                                                            13      0   100%
/actions-runner/_work/airbyte/airbyte/airbyte-integrations/bases/airbyte-protocol/airbyte_protocol/models/airbyte_protocol.py     124      0   100%
/actions-runner/_work/airbyte/airbyte/airbyte-integrations/bases/airbyte-protocol/airbyte_protocol/models/__init__.py               1      0   100%
/actions-runner/_work/airbyte/airbyte/airbyte-integrations/bases/airbyte-protocol/airbyte_protocol/__init__.py                      2      0   100%
base_python/cdk/utils/event_timing.py                                                                                              47      3    94%
base_python/cdk/streams/auth/core.py                                                                                                8      1    88%
base_python/cdk/streams/exceptions.py                                                                                              10      2    80%
base_python/cdk/streams/auth/token.py                                                                                               9      4    56%
base_python/logger.py                                                                                                              33     15    55%
base_python/cdk/streams/rate_limiting.py                                                                                           30     14    53%
base_python/integration.py                                                                                                         52     25    52%
base_python/cdk/streams/http.py                                                                                                    67     33    51%
base_python/cdk/streams/core.py                                                                                                    63     32    49%
base_python/client.py                                                                                                              56     33    41%
base_python/catalog_helpers.py                                                                                                     10      6    40%
base_python/source.py                                                                                                              51     34    33%
base_python/cdk/streams/auth/oauth.py                                                                                              37     26    30%
base_python/cdk/abstract_source.py                                                                                                 89     64    28%
base_python/schema_helpers.py                                                                                                      56     41    27%
base_python/entrypoint.py                                                                                                          70     56    20%
---------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                                                                                             832    389    53%
Name                          Stmts   Miss  Cover
-------------------------------------------------
source_airtable/__init__.py       2      0   100%
source_airtable/helpers.py       32      1    97%
source_airtable/source.py        74     22    70%
-------------------------------------------------
TOTAL                           108     23    79%

Python short test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.7/site-packages/source_acceptance_test/plugin.py:56: Skipping TestIncremental.test_two_sequential_reads because not found in the config
======================== 20 passed, 1 skipped in 17.14s ========================

@codecov
Copy link

codecov bot commented Apr 11, 2022

Codecov Report

❗ No coverage uploaded for pull request base (master@b85c513). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head a5b71fc differs from pull request most recent head 549441d. Consider uploading reports for the commit 549441d to get more accurate results

@@            Coverage Diff            @@
##             master   airbytehq/airbyte#11763   +/-   ##
=========================================
  Coverage          ?   77.90%           
=========================================
  Files             ?       86           
  Lines             ?     4385           
  Branches          ?        0           
=========================================
  Hits              ?     3416           
  Misses            ?      969           
  Partials          ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b85c513...549441d. Read the comment docs.

- uses: actions/setup-python@v2
with:
python-version: "3.8"
python-version: "3.9"
Copy link
Contributor

Choose a reason for hiding this comment

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

nice! heads up @alafanechere

python_version: '3.7.9'
pip_version: '21.1'
subdir: 'airbyte-cdk/python/'
python_version: "3.9.11"
Copy link
Contributor

@davinchia davinchia Apr 11, 2022

Choose a reason for hiding this comment

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

is it worth standardising on 3.9.11/3.9 everywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I have. Did I miss one?
Or are you saying not to change this one? I was following the hint on line 76 there.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was wondering out loud whether it's worth standardising the entire repo to 3.9.11 instead of having 3.9.11 here and 3.9 on the connectors.

Copy link
Contributor

@davinchia davinchia left a comment

Choose a reason for hiding this comment

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

@bleonard :shipit:

I left one non-blocking comment about standardising the python versions.

Looks like the main build is all green (doesn't include the connector builds), so master should be green as well!

@bleonard bleonard temporarily deployed to more-secrets April 11, 2022 18:14 Inactive
@bleonard bleonard temporarily deployed to more-secrets April 11, 2022 18:14 Inactive
@@ -1 +1 @@
3.8.12
3.9.11
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alafanechere I'm not sure if this file is needed or not now that they are the same. Can you follow up and check once this is merged?

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, I could now use 3.9.11 now indeed.

@bleonard bleonard temporarily deployed to more-secrets April 11, 2022 19:55 Inactive
@bleonard bleonard temporarily deployed to more-secrets April 11, 2022 19:55 Inactive
@bleonard bleonard temporarily deployed to more-secrets April 11, 2022 21:36 Inactive
@bleonard bleonard temporarily deployed to more-secrets April 11, 2022 21:36 Inactive
Previous example (source-google-search-console/credentials)
@bleonard bleonard temporarily deployed to more-secrets April 12, 2022 02:24 Inactive
@bleonard bleonard temporarily deployed to more-secrets April 12, 2022 02:24 Inactive
@bleonard bleonard temporarily deployed to more-secrets April 12, 2022 02:52 Inactive
@bleonard bleonard temporarily deployed to more-secrets April 12, 2022 02:52 Inactive
@bleonard bleonard temporarily deployed to more-secrets April 12, 2022 03:18 Inactive
@bleonard bleonard temporarily deployed to more-secrets April 12, 2022 03:18 Inactive
@bleonard bleonard merged commit c302af4 into master Apr 12, 2022
@bleonard bleonard deleted the bl-python-39 branch April 12, 2022 03:51
@noahkawasaki-airbyte
Copy link
Contributor

Is it desirable to have all connectors on one Python version @davinchia? I put this up a while ago in attempt to do what you brought up (bring standardization to versions) but got blocked from merging on past employer rules that no longer apply to me :D

#9230

I'll pick that up again and maybe add in a way to apply the python version to the Dockerfiles if that is wanted

@davinchia
Copy link
Contributor

@noahkawasaki-airbyte not a strict guideline, but definitely we should try our best to do so to keep things simple. I'll defer to @sherifnada on the exact details.

I remember your pr :)

@sherifnada
Copy link
Contributor

@noahkawasaki-airbyte I think it's desirable as long as there is a way to easily express exceptions e.g: there may be one straggler package which can't work with that version, in which case it needs to be trivial to switch that one over imo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation CDK Connector Development Kit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants