[BEAM-9073] Fixes order-dependence in PipelineVisitor

[rohdesamuel]

The Python PipelineVisitor is topologically-order dependent and can visit the same transform multiple times. The fix is to journal each transform we visit.

---

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.
• 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	Apex	Dataflow	Flink	Gearpump	Samza	Spark
Go		---	---		---	---
Java
Python		---			---	---
XLang	---	---	---		---	---	---

Pre-Commit Tests Status (on master branch)

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

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

[rohdesamuel]

R: @lukecwik can you review this please?

[lukecwik]

Wouldn't it make sense to ensure the topological visiting order in the graph instead of making the visit order possibly random?

(I ask this since some pipeline transform replacement algorithms may assume that you first visit the composites before you have visited the children so that replacing the higher level transform happens first).

[rohdesamuel]

Wouldn't it make sense to ensure the topological visiting order in the graph instead of making the visit order possibly random?

(I ask this since some pipeline transform replacement algorithms may assume that you first visit the composites before you have visited the children so that replacing the higher level transform happens first).

That makes sense, but unfortunately I don't have time to implement that larger change. I have a smaller work-around that fixes it at the PipelineVisitor subclass that I'm using.

[lukecwik]

Definitely an improvement since it prevents visiting the same transform multiple times but strongly consider following up with a better fix.

[lukecwik]

retest this please

[lukecwik]

Error Message
AttributeError: 'PipelineTest' object has no attribute 'assertCountEqual'
Stacktrace
self = <apache_beam.pipeline_test.PipelineTest testMethod=test_visitor_not_sorted>

[stale]

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions.

[stale]

This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.