[BEAM-13590] Fix abc imports from collections #15850

potiuk · 2021-10-30T17:39:09Z

In python 3.3 a number of classes (Abstract Base Clases) from "collections" have been
moved from "collections" to "collections.abc" and their use was deprecated.

See https://docs.python.org/3/library/collections.abc.html

Since Pythnon 3.7 warning started to be generated, that the imports will be removed
in 3.9. It did not happen till 3.10, however, it eventualy will so maybee worth fixing
If anything - just to get rid of the warning.

Beam uses Python 3.6+ so it is a safe change.

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.

`ValidatesRunner` compliance status (on master branch)

Lang	ULR	Twister2
Go	---	---
Java
Python	---	---
XLang		---

Examples testing status on various runners

Lang	ULR	Dataflow	Flink	Samza	Spark	Twister2
Go	---	---	---	---	---	---	---
Java	---		---	---	---	---	---
Python	---	---	---	---	---	---	---
XLang	---	---	---	---	---	---	---

Post-Commit SDK/Transform Integration Tests Status (on master branch)

Go	Java	Python

Pre-Commit Tests Status (on master branch)

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

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

GitHub Actions Tests Status (on master branch)

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

potiuk · 2021-10-30T17:40:14Z

cc: @pabloem @udim @lazylynx - this time with the right "base" 🤦

codecov · 2021-10-30T17:56:27Z

Codecov Report

Merging #15850 (07e60d7) into master (8724555) will increase coverage by 37.29%.
The diff coverage is 90.90%.

@@             Coverage Diff             @@
##           master   #15850       +/-   ##
===========================================
+ Coverage   46.32%   83.62%   +37.29%     
===========================================
  Files         201      452      +251     
  Lines       19733    62067    +42334     
===========================================
+ Hits         9142    51902    +42760     
- Misses       9610    10165      +555     
+ Partials      981        0      -981

Impacted Files	Coverage Δ
sdks/python/apache_beam/transforms/trigger.py	`89.87% <50.00%> (ø)`
.../apache_beam/runners/direct/transform_evaluator.py	`90.32% <100.00%> (ø)`
...ks/python/apache_beam/runners/worker/sideinputs.py	`87.96% <100.00%> (ø)`
sdks/python/apache_beam/typehints/typecheck.py	`97.54% <100.00%> (ø)`
sdks/python/apache_beam/typehints/typehints.py	`93.43% <100.00%> (ø)`
sdks/go/pkg/beam/core/runtime/harness/session.go
sdks/go/pkg/beam/core/runtime/coderx/varint.go
sdks/go/pkg/beam/testing/passert/sum.go
sdks/go/pkg/beam/xlang.go
sdks/go/pkg/beam/core/runtime/exec/encode.go
... and 648 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 8724555...07e60d7. Read the comment docs.

After seting 3.7 the default (apache#19317) the warning printed by Python during importing all providers (specifically apache beam) has slightly changed. Apparently collections.abc warning was a bit more "scary" - warning that it's 3.9 not 3.10 where the old collection imports will stop working (Note that actually this did not happen even in 3.10, apparently) This PR fixes the "known" warning message to match it but also a separate PR (apache/beam#15850) was opened to Beam to get rid of the warnings altogether. Also seems 'dns` stopped generating this warning so I removed it and in case warnings are generated, they are printed outside of the folded group, so that it's immediately visible.

After seting 3.7 the default (#19317) the warning printed by Python during importing all providers (specifically apache beam) has slightly changed. Apparently collections.abc warning was a bit more "scary" - warning that it's 3.9 not 3.10 where the old collection imports will stop working (Note that actually this did not happen even in 3.10, apparently) This PR fixes the "known" warning message to match it but also a separate PR (apache/beam#15850) was opened to Beam to get rid of the warnings altogether. Also seems 'dns` stopped generating this warning so I removed it and in case warnings are generated, they are printed outside of the folded group, so that it's immediately visible.

udim · 2021-11-01T16:24:56Z

Hi, thanks for the contribution!

I think you need from collections.abc import Sequence, at least for Python 3.8:

>>> import collections
>>> collections.abc.Sequence
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/google/home/ehudm/.pyenv/versions/3.8.12/lib/python3.8/collections/__init__.py", line 55, in __getattr__
    raise AttributeError(f'module {__name__!r} has no attribute {name!r}')
AttributeError: module 'collections' has no attribute 'abc'

udim · 2021-11-01T16:28:17Z

Or maybe: import collections.abc. Honestly, I'm not sure how the existing code works with collections.abc.Iterable.

pabloem · 2021-11-04T17:28:43Z

what are next steps here?

potiuk · 2021-11-04T20:24:19Z

Or maybe: import collections.abc. Honestly, I'm not sure how the existing code works with collections.abc.Iterable.

Yeah. I wonder how all tests are passing this one :).

potiuk · 2021-11-04T20:28:46Z

Updatd it - I can also squash it if you are not squashing during the merge.

However it is a bit worrying that previous commit did not fail any tests :(

In python 3.3 a number of classes (Abstract Base Clases) from "collections" have been moved from "collections" to "collections.abc" and their use was deprecated. See https://docs.python.org/3/library/collections.abc.html Since Pythnon 3.7 warning started to be generated, that the imports will be removed in 3.9. It did not happen till 3.10, however, it eventualy will so maybee worth fixing If anything - just to get rid of the warning.

potiuk · 2021-11-07T20:16:37Z

I also fixed pylint issues - should be good to go :)

udim · 2021-11-09T18:07:04Z

Thanks! Should be good to submit once python precommit passes.
I imagine there was something else that was already importing the collections.abc module (pytest perhaps?).

potiuk · 2021-11-09T19:41:04Z

Actually @udim - I found that there are few other (internal, not exposed to users) uses of Iterable/Sequence so I corrected it in a fixup. I also scanned whole Python code to see if there are no other collection.abcs (Generator, MutableSet and the like) and I have not found any.

tvalentyn · 2022-01-14T20:10:16Z

PythonDocker precommit error is unrelated and addressed (https://lists.apache.org/thread/87kjxw384c2pdh953vojsc10jzfs2rzf). This PR does not affect that suite.

potiuk · 2022-01-14T20:41:46Z

Cool!

* github/master: (198 commits) Merge pull request apache#16369 from [BEAM-13558] [Playground] Hide the Graph tab and SCIO from SDK options Merge pull request apache#16546 from [BEAM-13661] [BEAM-13704] [Playground] Update tags for examples/katas/unit-tests Merge pull request apache#16540 from [BEAM-13678][Playground]Update Github Action To Deploy Examples [BEAM-13430] Re-add provided configuration (apache#16552) [BEAM-10206] Remove Fatalf calls in non-test goroutines for tests/benchmarks (apache#16575) [BEAM-13693] Bump beam_PostCommit_Java_ValidatesRunner_Dataflow_Streaming timeout to 12 hours (apache#16576) [BEAM-13699] Replace fnv with maphash. (apache#16573) Update Java FnAPI beam master (apache#16572) Merge pull request apache#16371 from [BEAM-13518][Playground] Beam Playground quickstart page on the Beam website Merge pull request apache#16373 from [BEAM-13515] [Playground] Hiding lines in an example that are not necessary Merge pull request apache#16472: [BEAM-13697] Add SchemaFieldNumber annotation [BEAM-13689] Output token elements when BQ batch writes complete. Disable logging for memoization test. (apache#16556) BEAM-13611 reactivating jdbcio xlang test Revert "Merge pull request apache#15863 from [BEAM-13184] Autosharding for JdbcIO.write* transforms" Remove obsolete commands from Inventory job. (apache#16564) [BEAM-13688] fixed type in BPG 4.5.3 window section (apache#16560) [BEAM-13683] Make cross-language SQL example pipeline (apache#16567) Merge pull request apache#16243 from darshan-sj/feature/support-priority-spannerio - Making rpcPriority a ValueProvider in SpannerConfig edited README and comments in Python multi-lang pipes examples Merge pull request apache#16518 from [BEAM-13619] [Playground] Add loading animation to the catalog Merge pull request apache#16519 from [BEAM-13639] [Playground] Add notification to Log/Output tabs about cached example Merge pull request apache#16533 from [BEAM-13548] [Playground] Add example description popover Merge pull request apache#16531 from [BEAM-13567] [playground] Handle run code validation and preparation errors Merge pull request apache#16370 from [BEAM-13556] playground - color and scroll tabs with new content [BEAM-13611] Skip test_xlang_jdbc_write (apache#16554) [BEAM-13015] Provide caching statistics in the status client. (apache#16495) Merge pull request apache#16309: [BEAM-13503] Set a default value to throwWriteErrors in BulkIO constructor [BEAM-13015] Add state caching capability to be used as hint for runners to not duplicate cached data if the SDK can do it for user state and side inputs. (apache#16525) [BEAM-13665] Make SpannerIO projectID optional again (apache#16547) Merge pull request apache#16322 from [BEAM-13407] [Playground] Preload fonts for the web application Merge pull request apache#16506 from [BEAM-13652][Playground] Send examples' links to the frontend [BEAM-11808][BEAM-9879] Support aggregate functions with two arguments (apache#16200) Update walkthrough.md (apache#16512) [BEAM-13683] Correct SQL transform schema, fix expansion address override bug (apache#16551) Merge pull request apache#16486 from [BEAM-13544][Playground] Add logs to examples CI/CD to see the progress [BEAM-13616][BEAM-13646] Upgrade vendored calcite to 1.28.0:0.2 (apache#16544) [BEAM-13616][BEAM-13645] Switch to vendored grpc 1.43.2 (apache#16543) Also bump FnAPI container. [BEAM-13680] Fixed code_repository (added pipelineUuid to RunCodeResult when status is "Finished") [BEAM-13616] Update com.google.cloud:libraries-bom to 24.2.0 (apache#16509) [BEAM-13430] Remove jcenter which will no longer contain any updates. (apache#16536) Update GH Actions to use proper variables names and proper triggers Remove jcenter repositories from gradle configuration. (apache#16532) Merge pull request apache#16507: [BEAM-13137] Fixes ES utest size flakiness with _flush api and index.store.stats_refresh_interval=0 [BEAM-13664] Fix Primitives hashing benchmark (apache#16523) Bump beam container version. [BEAM-12621] - Update Jenkins VMs to modern Ubuntu version (apache#16457) doc tweaks (apache#16498) Redirecting cross-language transforms content (apache#16504) Remove tab from source. Remove unnecessary fmt call in universal.go Clean up string cast of bytes in vet.go and corresponding tests fix capitalized error strings in expansionx Remove unnecessary blank identifier assignment in harness.go Replace string(buf.Bytes()) with buf.String() in coder_test.go Replace bytes.Compare() with bytes.Equal() in test cases Remove unnecessary fmt.Sprintf() in partition.go Fix staticcheck errors in transforms directory [BEAM-13590] Fix abc imports from collections (apache#15850) Merge pull request apache#16482 from [BEAM-13429][Playground] Add builder for preparers [BEAM-10206] Resolve go vet errors in protox package [BEAM-12572] Run java examples on multiple runners (apache#16450) [BEAM-13400] JDBC IO does not support UUID and JSONB PostgreSQL types and OTHER JDBC types in general [BEAM-13577] Beam Select's uniquifyNames function loses nullability of Complex types while inferring schema [BEAM-12164]: Add SDF for reading change stream records [BEAM-13455] Remove duplicated artifacts when using multiple environments with Dataflow Java Merge pull request apache#16485 from [BEAM-13486] [Playground] For unit tests (java) if one of tests fails the output goes to stdOutput Merge pull request apache#16385 from [BEAM-13535] [Playground] add cancel execution button [BEAM-12558] Fix doc typo. Merge pull request apache#16467 from [BEAM-12164]: SpannerIO DetectNewPartitions SDF Introduce the notion of a JoinIndex for fewer shuffles. (apache#16101) [BEAM-12464] Change ProtoSchemaTranslator beam schema creation to match the order for protobufs containing Oneof fields (apache#14974) Stronger typing inference for CoGBK. (apache#16465) [BEAM-13480] Increase pipeline timeout for PubSubIntegrationTest.test_streaming_data_only (apache#16496) Provide API to check whether a hint is known. [BEAM-8806] Integration test for SqsIO using Localstack Split builder into several builder for each step of pipeline execution [BEAM-13399] Move service liveness polling to Runner type (apache#16487) Merge pull request apache#16325 from [BEAM-13471] [Playground] Tag existing unit-tests Adds several example multi-language Python pipelines [BEAM-13616][BEAM-13646] Update vendored calcite 1.28.0 with protobuf 3.19.2 (apache#16473) Merge pull request apache#16374 from [BEAM-13398][Playground] Split LifeCycle to DTO and business logic Merge pull request apache#16363 from [BEAM-13557] [Playground] show code execution time Merge pull request apache#16149 from [BEAM-13113] [Playground] playground frontend documentation Merge pull request apache#16469 from [BEAM-13623][Playground] [Bugfix] During unit tests failing there is no any output [BEAM-13641][Playground] Add SCIO SDK support on the CI/CD step [BEAM-13638] Datatype of timestamp fields in SqsMessage for AWS IOs for SDK v2 was changed from String to long, visibility of all fields was fixed from package private to public [BEAM-13616] Initial files for vendored gRPC 1.43.2 (apache#16460) [BEAM-13432] Skip ExpansionService creation in Job Server (apache#16222) [BEAM-13628] Update SideInputCache to use full Transform and SideInputIDs as token information (apache#16483) [BEAM-13631] Add deterministic SQS message coder to fix reading from SQS in batch mode [BEAM-8806] Integration test for SqsIO [adhoc] Run spotlessApply on java examples to fix master Merge pull request apache#16156 from [BEAM-13391] Fix temporary file format in WriteToBigQuery Optional args and kwargs for named external transforms. [BEAM-13614] Add OnWindowExpiration support to the Java SDK harness and proto translation. (apache#16458) [BEAM-3221] Improve documentation in model pipeline protos (apache#16474) Merge pull request apache#16147 from [BEAM-13359] [Playground] Tag existing examples [BEAM-13626] Remap expanded outputs after merging. (apache#16471) ...

After seting 3.7 the default (#19317) the warning printed by Python during importing all providers (specifically apache beam) has slightly changed. Apparently collections.abc warning was a bit more "scary" - warning that it's 3.9 not 3.10 where the old collection imports will stop working (Note that actually this did not happen even in 3.10, apparently) This PR fixes the "known" warning message to match it but also a separate PR (apache/beam#15850) was opened to Beam to get rid of the warnings altogether. Also seems 'dns` stopped generating this warning so I removed it and in case warnings are generated, they are printed outside of the folded group, so that it's immediately visible. (cherry picked from commit 4bb1317)

After seting 3.7 the default (#19317) the warning printed by Python during importing all providers (specifically apache beam) has slightly changed. Apparently collections.abc warning was a bit more "scary" - warning that it's 3.9 not 3.10 where the old collection imports will stop working (Note that actually this did not happen even in 3.10, apparently) This PR fixes the "known" warning message to match it but also a separate PR (apache/beam#15850) was opened to Beam to get rid of the warnings altogether. Also seems 'dns` stopped generating this warning so I removed it and in case warnings are generated, they are printed outside of the folded group, so that it's immediately visible. (cherry picked from commit 4bb1317c892c6d65557b3d998bd8d1bd971ba96b) GitOrigin-RevId: ae184690fd27cc3f549dc427d9d70cc1c10020da

After seting 3.7 the default (#19317) the warning printed by Python during importing all providers (specifically apache beam) has slightly changed. Apparently collections.abc warning was a bit more "scary" - warning that it's 3.9 not 3.10 where the old collection imports will stop working (Note that actually this did not happen even in 3.10, apparently) This PR fixes the "known" warning message to match it but also a separate PR (apache/beam#15850) was opened to Beam to get rid of the warnings altogether. Also seems 'dns` stopped generating this warning so I removed it and in case warnings are generated, they are printed outside of the folded group, so that it's immediately visible. GitOrigin-RevId: 4bb1317c892c6d65557b3d998bd8d1bd971ba96b

potiuk mentioned this pull request Oct 30, 2021

Update known warnings for Python 3.7 apache/airflow#19333

Merged

potiuk force-pushed the fix-collection-imports branch from 57edfa0 to ece1d10 Compare November 7, 2021 15:26

potiuk force-pushed the fix-collection-imports branch from ece1d10 to d02d82a Compare November 7, 2021 20:16

pabloem requested a review from udim November 9, 2021 18:05

undo change (unrelated?)

5c018ff

udim approved these changes Nov 9, 2021

View reviewed changes

fixup! undo change (unrelated?)

3dae57c

Merge branch 'master' into fix-collection-imports

19e2b45

tvalentyn mentioned this pull request Jan 14, 2022

[BEAM-13590] Renaming collections.Iterable to collections.abc.Iterable. #16414

Closed

tvalentyn changed the title ~~Fix abc imports from collections~~ [BEAM-13590] Fix abc imports from collections Jan 14, 2022

Yapf

07e60d7

tvalentyn approved these changes Jan 14, 2022

View reviewed changes

tvalentyn merged commit 9d40a4c into apache:master Jan 14, 2022

tvalentyn mentioned this pull request Jan 14, 2022

[BEAM-12000] Update deprecated collections to use collections.abc in python sdk #15415

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BEAM-13590] Fix abc imports from collections #15850

[BEAM-13590] Fix abc imports from collections #15850

potiuk commented Oct 30, 2021 •

edited

Loading

potiuk commented Oct 30, 2021

codecov bot commented Oct 30, 2021 •

edited

Loading

udim commented Nov 1, 2021

udim commented Nov 1, 2021 •

edited

Loading

pabloem commented Nov 4, 2021

potiuk commented Nov 4, 2021

potiuk commented Nov 4, 2021

potiuk commented Nov 7, 2021

udim commented Nov 9, 2021

potiuk commented Nov 9, 2021

tvalentyn commented Jan 14, 2022

potiuk commented Jan 14, 2022

[BEAM-13590] Fix abc imports from collections #15850

[BEAM-13590] Fix abc imports from collections #15850

Conversation

potiuk commented Oct 30, 2021 • edited Loading

ValidatesRunner compliance status (on master branch)

Examples testing status on various runners

Post-Commit SDK/Transform Integration Tests Status (on master branch)

Pre-Commit Tests Status (on master branch)

GitHub Actions Tests Status (on master branch)

potiuk commented Oct 30, 2021

codecov bot commented Oct 30, 2021 • edited Loading

Codecov Report

udim commented Nov 1, 2021

udim commented Nov 1, 2021 • edited Loading

pabloem commented Nov 4, 2021

potiuk commented Nov 4, 2021

potiuk commented Nov 4, 2021

potiuk commented Nov 7, 2021

udim commented Nov 9, 2021

potiuk commented Nov 9, 2021

tvalentyn commented Jan 14, 2022

potiuk commented Jan 14, 2022

potiuk commented Oct 30, 2021 •

edited

Loading

`ValidatesRunner` compliance status (on master branch)

codecov bot commented Oct 30, 2021 •

edited

Loading

udim commented Nov 1, 2021 •

edited

Loading