robertwb commented on code in PR #30003: URL: https://github.com/apache/beam/pull/30003#discussion_r1454233006
########## sdks/python/apache_beam/yaml/docs/README.md: ########## @@ -0,0 +1,295 @@ +# Beam YAML Developer Documentation Review Comment: I think this level of documentation (for developers) is best kept as a living doc (which you already have) rather than checked into the repo. ########## sdks/python/apache_beam/yaml/examples/test/map_to_fields_callable_test.py: ########## @@ -0,0 +1,36 @@ +# coding=utf-8 Review Comment: Having a separate test file for (nearly) every example seems overkill. Perhaps we could consolidate? ########## sdks/python/apache_beam/yaml/examples/README.md: ########## @@ -0,0 +1,48 @@ +# Examples Catalog + +<!-- TOC --> +* [Examples Catalog](#examples-catalog) Review Comment: Long term (post 2.54) I we'll want these to live in the beam website, not here. But we can move them then. ########## sdks/python/apache_beam/yaml/examples/transforms/aggregation/combine_sum.yaml: ########## @@ -0,0 +1,58 @@ +# coding=utf-8 +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +pipeline: + type: chain + transforms: + - type: Create + name: Create produce + config: + elements: + - recipe: 'pie' + fruit: 'raspberry' + quantity: 1 + unit_price: 3.50 + - recipe: 'pie' + fruit: 'blackberry' + quantity: 1 + unit_price: 4.00 + - recipe: 'pie' + fruit: 'blueberry' + quantity: 1 + unit_price: 2.00 + - recipe: 'muffin' + fruit: 'blueberry' + quantity: 2 + unit_price: 2.00 + - recipe: 'muffin' + fruit: 'banana' + quantity: 3 + unit_price: 1.00 + - type: Combine + name: Sum values per key + config: + language: python + group_by: fruit + combine: + total_quantity: Review Comment: Let's make this example combine more than just one field. ########## sdks/python/apache_beam/yaml/examples/wordcount_minimal.yaml: ########## @@ -0,0 +1,52 @@ +# coding=utf-8 +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +pipeline: + type: chain + transforms: + - type: ReadFromText + config: + path: gs://dataflow-samples/shakespeare/kinglear.txt Review Comment: Does one need credentials to access this public file? (If so, add a comment.) ########## sdks/python/apache_beam/yaml/examples/transforms/elementwise/explode.yaml: ########## @@ -0,0 +1,31 @@ +# coding=utf-8 +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +pipeline: + type: chain + transforms: + - type: Create + name: Gardening plants + config: + elements: + - produce: ['🍓Strawberry', '🥕Carrot', '🍆Eggplant', '🍅Tomato', '🥔Potato'] Review Comment: Not sure how clear this example is. Maybe add a non-exploding field? ########## sdks/python/apache_beam/yaml/examples/test/regex_matches_yaml_test.py: ########## @@ -0,0 +1,41 @@ +# coding=utf-8 +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# pytype: skip-file + +import unittest + +from apache_beam.yaml.examples.test.test_yaml_example import test_yaml_example + +YAML_FILE = '../regex_matches.yaml' + +EXPECTED = [ Review Comment: Actually, it looks like these are all *very* formulaic. What if we put a comment `# Expected: ...` right in the yaml file itself? This would be useful to the reader of the yaml file, and would obviate the need for all these separate test files (we would just do a glob for *.yaml in the directories). We could alternatively add an "expected" parameter to LogForTesting (which would do its verification via AssertThat, no need to even scrape stdout) if we didn't want to parse it out ourselves. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
