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

Run test for Python-3.11 #1237

Merged
merged 8 commits into from
Aug 2, 2023
Merged

Run test for Python-3.11 #1237

merged 8 commits into from
Aug 2, 2023

Conversation

pankajastro
Copy link
Contributor

@pankajastro pankajastro commented Jul 11, 2023

Run unit tests for Python3.11
exclude hive extra from it because of issue cloudera/python-sasl#30

@pankajastro
Copy link
Contributor Author

CI failed while building sasl mostly CircleCI python env. I'll investigate it further.

@pankajastro pankajastro force-pushed the py-3-11 branch 13 times, most recently from 3035a3a to 7313d30 Compare July 12, 2023 14:47
Copy link
Collaborator

@pankajkoti pankajkoti left a comment

Choose a reason for hiding this comment

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

Looks like some checks are failing too (mypy and pre-commit check)

setup.cfg Show resolved Hide resolved
@pankajastro pankajastro force-pushed the py-3-11 branch 2 times, most recently from 5167ff2 to f181cd8 Compare July 14, 2023 12:43
@pankajastro
Copy link
Contributor Author

CI failed while building sasl mostly CircleCI python env. I'll investigate it further.

This known issue cloudera/python-sasl#30

@pankajastro pankajastro marked this pull request as ready for review July 14, 2023 12:45
setup.cfg Show resolved Hide resolved
@pankajastro pankajastro requested a review from pankajkoti July 14, 2023 12:48
@mdeshmu
Copy link

mdeshmu commented Jul 14, 2023

@pankajastro

I made a couple of contributions to PyHive which were accepted and released in 0.7.1.dev0. You are requested to test with the dev version and report any bugs in the PyHive GitHub repository before 0.7.1 is released in a month or so.

  1. PyHive also supports pure-sasl via additional extras 'pyhive[hive_pure_sasl]' which supports Python 3.11 in addition to previous Python versions. See Use pure-sasl in python 3.11 dropbox/PyHive#454
  2. PyHive is now compatible with SQLAlchemy 2.0. See Adding compatibility with SQLAlchemy 2.0 dropbox/PyHive#457

@pankajastro
Copy link
Contributor Author

@pankajastro

I made a couple of contributions to PyHive which were accepted and released in 0.7.1.dev0. You are requested to test with the dev version and report any bugs in the PyHive GitHub repository before 0.7.1 is released in a month or so.

  1. PyHive also supports pure-sasl via additional extras 'pyhive[hive_pure_sasl]' which supports Python 3.11 in addition to previous Python versions. See Use pure-sasl in python 3.11 dropbox/PyHive#454
  2. PyHive is now compatible with SQLAlchemy 2.0. See Adding compatibility with SQLAlchemy 2.0 dropbox/PyHive#457

Thanks @mdeshmu for letting us know. We are having this issue because we depend on the airflow apache hive provider that depends on sasl. Look like Jarek has already opened PR apache/airflow#32607 for airflow provider testing

@codecov
Copy link

codecov bot commented Jul 25, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (64dea53) 98.57% compared to head (bd32dc5) 98.57%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1237   +/-   ##
=======================================
  Coverage   98.57%   98.57%           
=======================================
  Files          90       90           
  Lines        5393     5393           
=======================================
  Hits         5316     5316           
  Misses         77       77           

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

Copy link
Collaborator

@pankajkoti pankajkoti left a comment

Choose a reason for hiding this comment

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

Mostly, looks good to me.

I am checking the results of Python 3.11 test job https://app.circleci.com/pipelines/github/astronomer/astronomer-providers/6082/workflows/fa610ca9-085d-4572-b550-4cefd117fba5/jobs/40521

Did a couple of tests fail and yet it reported a success? Can we please check this?

glob "tests/**/test_*.py" | sed '/tests\/apache\/hive/d' \
| circleci tests split --split-by=timings \
)
pytest --junit-xml=test-report/report.xml $TEST_FILES
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add some comment (maybe at the top of the step?) on why we are ignoring to include the code coverage here?

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 have added on at the top of this job PTAL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aha, I missed the codecov step. I did not include it because except for hive all tests are running here and including it will not change coverage. But happy to include it if you think it makes sense

.circleci/scripts/pre_commit_readme_extra.py Show resolved Hide resolved
setup.cfg Show resolved Hide resolved
@pankajkoti
Copy link
Collaborator

okay, looks good to me then when the above comments are addressed, great work 👏🏽

@pankajastro
Copy link
Contributor Author

not sure, what happened I just did the force pushed and closed PR 🤔

@pankajastro
Copy link
Contributor Author

re-opening

@pankajkoti
Copy link
Collaborator

It says PR closed, but PR merged and closed :D

Screenshot 2023-08-01 at 6 09 39 PM

@pankajastro pankajastro reopened this Aug 1, 2023
@pankajastro
Copy link
Contributor Author

@pankajkoti yes something I messed up :)

@pankajastro pankajastro requested review from Lee-W and pankajkoti August 1, 2023 15:30
pankajastro and others added 2 commits August 2, 2023 11:58
Co-authored-by: Wei Lee <weilee.rx@gmail.com>
Copy link
Collaborator

@pankajkoti pankajkoti left a comment

Choose a reason for hiding this comment

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

Looks good to me but can't seem to find codecov comment #1237 (comment)

@pankajastro
Copy link
Contributor Author

Looks good to me but can't seem to find codecov comment #1237 (comment)

I was referring to this comment file .circleci/config.yml

# This runs unit tests for Python3.11 only and excludes the Apache Hive test since
# Currently, the Airflow Hive provider is excluded from Python3.11 in Airflow
# Check the issue https://github.com/cloudera/python-sasl/issues/30 for detail
# PR to bring back Airflow Apache Hive provider https://github.com/apache/airflow/pull/32607

.circleci/config.yml Outdated Show resolved Hide resolved
@pankajastro pankajastro merged commit 83957b8 into main Aug 2, 2023
@pankajastro pankajastro deleted the py-3-11 branch August 2, 2023 16:51
pankajastro added a commit that referenced this pull request Aug 29, 2023
pankajastro added a commit that referenced this pull request Aug 29, 2023
* Bring back hive provider for Python3.11

* Revert #1237

* Bump hive provider verion

* Update config
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants