Skip to content

[BEAM-7372] remove codepath and workaround for py2 from io#14292

Merged
tvalentyn merged 2 commits into
apache:masterfrom
yoshikiobata:cleanup_py2_codepath_from_io
Mar 25, 2021
Merged

[BEAM-7372] remove codepath and workaround for py2 from io#14292
tvalentyn merged 2 commits into
apache:masterfrom
yoshikiobata:cleanup_py2_codepath_from_io

Conversation

@yoshikiobata
Copy link
Copy Markdown
Contributor

@yoshikiobata yoshikiobata commented Mar 21, 2021

Removed codepath for python2 with checking sys.version_info and also workarounds for it.
Usage of future package will be removed in another PR.


Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Choose reviewer(s) and mention them in a comment (R: @username).
  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
  • Update CHANGES.md with noteworthy changes.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

Post-Commit Tests Status (on master branch)

Lang SDK Dataflow Flink Samza Spark Twister2
Go Build Status --- Build Status --- Build Status ---
Java Build Status Build Status
Build Status
Build Status
Build Status
Build Status
Build Status
Build Status
Build Status Build Status
Build Status
Build Status
Build Status
Python Build Status
Build Status
Build Status
Build Status
Build Status
Build Status
Build Status
Build Status
--- Build Status ---
XLang Build Status Build Status Build Status --- Build Status ---

Pre-Commit Tests Status (on master branch)

--- Java Python Go Website Whitespace Typescript
Non-portable Build Status Build Status
Build Status
Build Status
Build Status
Build Status Build Status Build Status Build Status
Portable --- Build Status --- --- --- ---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

GitHub Actions Tests Status (on master branch)

Build python source distribution and wheels
Python tests
Java tests

See CI.md for more information about GitHub Actions CI.

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 21, 2021

Codecov Report

Merging #14292 (a21fa10) into master (3930d12) will increase coverage by 0.41%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14292      +/-   ##
==========================================
+ Coverage   82.97%   83.39%   +0.41%     
==========================================
  Files         469      469              
  Lines       58343    58727     +384     
==========================================
+ Hits        48411    48973     +562     
+ Misses       9932     9754     -178     
Impacted Files Coverage Δ
sdks/python/apache_beam/io/gcp/pubsub.py
sdks/python/apache_beam/transforms/external.py
sdks/python/apache_beam/io/vcfio.py
sdks/python/apache_beam/io/snowflake.py
sdks/python/apache_beam/testing/__init__.py
sdks/python/apache_beam/dataframe/frame_base.py
sdks/python/apache_beam/io/kinesis.py
...pache_beam/portability/api/beam_fn_api_pb2_grpc.py
...e_beam/io/gcp/internal/clients/storage/__init__.py
sdks/python/apache_beam/transforms/timeutil.py
... and 922 more

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 3930d12...a21fa10. Read the comment docs.

@yoshikiobata yoshikiobata force-pushed the cleanup_py2_codepath_from_io branch from 8b7b24c to 0e51f56 Compare March 22, 2021 12:41
@yoshikiobata yoshikiobata marked this pull request as ready for review March 22, 2021 13:47
@yoshikiobata
Copy link
Copy Markdown
Contributor Author

R: @tvalentyn

@yoshikiobata
Copy link
Copy Markdown
Contributor Author

yoshikiobata commented Mar 22, 2021

Curiously codecov told that coverage is 0.00%, but actually 83.40% according to the report.

Copy link
Copy Markdown
Contributor

@tvalentyn tvalentyn left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

# Avro RecordSchema provides field_map in py3 and fields_dict in py2
field_map = getattr(parsed_schema, "field_map", None) or \
getattr(parsed_schema, "fields_dict", None)
field_map = getattr(parsed_schema, "field_map", None)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can eliminate this variable

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated

@tvalentyn
Copy link
Copy Markdown
Contributor

I don't understand coverage error. If you want you can try to reproduce it on a smaller change. I'm happy to merge as is.

@yoshikiobata
Copy link
Copy Markdown
Contributor Author

Hmm, coverage check seems not to run.
Do not hesitate to merge if changes a21fa10 are OK

@tvalentyn tvalentyn merged commit 7563928 into apache:master Mar 25, 2021
@yoshikiobata yoshikiobata deleted the cleanup_py2_codepath_from_io branch April 7, 2021 12:26
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.

2 participants