[yaml] Add yaml_provider.py Unit Tests #27804

Polber · 2023-08-02T16:45:02Z

Add unit tests for yaml_provider.py.

This PR also introduces non-breaking name changes to functions and variables to suppress IDE warnings.

There is also a change to the WithSchema built-in transform - it now requires a schema tag so that the schema field names do not collide with the top-level transform tags (e.g "name").

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

Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
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.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.

codecov · 2023-08-02T17:49:48Z

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (1032533) 72.31% compared to head (6eea11a) 72.34%.
Report is 1245 commits behind head on master.

Files	Patch %	Lines
sdks/python/apache_beam/yaml/yaml_provider.py	72.72%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #27804      +/-   ##
==========================================
+ Coverage   72.31%   72.34%   +0.03%     
==========================================
  Files         683      683              
  Lines      100633   100640       +7     
==========================================
+ Hits        72768    72813      +45     
+ Misses      26289    26251      -38     
  Partials     1576     1576

Flag	Coverage Δ
python	`82.88% <72.72%> (+0.06%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

github-actions · 2023-08-02T19:06:10Z

Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment assign set of reviewers

sdks/python/apache_beam/yaml/yaml_provider_ut_test.py

Polber · 2023-09-12T23:54:13Z

sdks/python/apache_beam/yaml/yaml_provider.py

+    if 'schema' not in args.keys():
+      raise ValueError("WithSchema transform missing required 'schema' tag.")
+    named_tuple = schemas.named_tuple_from_schema(parse_schema(args['schema']))


@robertwb Should I remove this now that we have the config tag?

The schema needs to be in the config now.

github-actions · 2023-09-13T01:27:25Z

Assigning reviewers. If you would like to opt out of this review, comment assign to next reviewer:

R: @liferoad for label python.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

Signed-off-by: Jeffrey Kinard <[email protected]>

robertwb · 2023-09-18T17:18:34Z

sdks/python/apache_beam/yaml/yaml_provider.py

+    try:
+      return self._transform_factories[type](**args)
+    except KeyError:
+      raise KeyError(f'Invalid transform specified: "{type}".')


There may be other key errors that we catch here. Also, this should never be called unless type is already in self._transform_factories. If we want to raise this error here, do an explicit check for the key in the dictionary, rather than catch all key errors that may occur in the call.

robertwb · 2023-09-18T17:19:14Z

sdks/python/apache_beam/yaml/yaml_provider.py

+      return self._transform_factories[type](**args)
+    except KeyError:
+      raise KeyError(f'Invalid transform specified: "{type}".')
+    except TypeError:


We already attempt a better error message at a higher level. But this will catch all type errors, not just invalid transform arguments.

robertwb · 2023-09-18T17:19:39Z

sdks/python/apache_beam/yaml/yaml_provider.py

@@ -372,7 +377,14 @@ def provided_transforms(self):
    return self._transform_factories.keys()

  def create_transform(self, type, args, yaml_create_transform):
-    return self._transform_factories[type](**args)
+    try:


I think we should revert catching these here (see below).

robertwb · 2023-09-18T17:27:51Z

sdks/python/apache_beam/yaml/yaml_provider_unit_test.py

+    with self.assertRaises(ValueError) as e:
+      ExternalProvider.provider_from_spec(spec)
+      self.assertEqual(beam_version, spec['version'])
+    self.assertRegex(str(e.exception), ".*Unexpected parameters in provider.*")


Use assertRaisesRegex?

robertwb · 2023-09-18T17:28:27Z

sdks/python/apache_beam/yaml/yaml_provider_unit_test.py

+      self.assertEqual(beam_version, spec['version'])
+    self.assertRegex(str(e.exception), ".*Unexpected parameters in provider.*")
+
+  # TODO - this test will need to be updated when docker support is added.


I'd just remove this test.

robertwb · 2023-09-18T17:29:06Z

sdks/python/apache_beam/yaml/yaml_provider_unit_test.py

+    }
+    with self.assertRaises(ValueError) as e:
+      ExternalProvider.provider_from_spec(spec)
+    self.assertRegex(str(e.exception), ".*Missing type in provider.*")


assertRaisesRegex

robertwb · 2023-09-18T18:02:45Z

sdks/python/apache_beam/yaml/yaml_provider_unit_test.py

+  def __init__(self, method_name="runExternalPythonProviderTest"):
+    super().__init__(method_name)
+
+    self._type = 'pythonPackage'


You could have _type as a class-level attribute and not bother overriding init.

github-actions · 2023-09-26T12:14:10Z

Reminder, please take a look at this pr: @liferoad

github-actions · 2023-09-29T12:14:13Z

Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment assign to next reviewer:

R: @AnandInguva for label python.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

github-actions · 2023-10-07T12:14:02Z

Reminder, please take a look at this pr: @AnandInguva

github-actions · 2023-12-06T12:41:39Z

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 [email protected] list. Thank you for your contributions.

AnandInguva · 2023-12-06T13:12:13Z

@Polber is this PR still relevant?

github-actions · 2024-02-06T12:39:22Z

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 [email protected] list. Thank you for your contributions.

github-actions · 2024-04-14T14:10:10Z

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 [email protected] list. Thank you for your contributions.

codecov-commenter · 2024-04-14T14:10:39Z

Codecov Report

Attention: Patch coverage is 72.72727% with 6 lines in your changes missing coverage. Please review.

Project coverage is 72.34%. Comparing base (1032533) to head (6eea11a).
Report is 3489 commits behind head on master.

Files with missing lines	Patch %	Lines
sdks/python/apache_beam/yaml/yaml_provider.py	72.72%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #27804      +/-   ##
==========================================
+ Coverage   72.31%   72.34%   +0.03%     
==========================================
  Files         683      683              
  Lines      100633   100640       +7     
==========================================
+ Hits        72768    72813      +45     
+ Misses      26289    26251      -38     
  Partials     1576     1576

Flag	Coverage Δ
python	`82.88% <72.72%> (+0.06%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

github-actions · 2024-06-14T12:50:07Z

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 [email protected] list. Thank you for your contributions.

github-actions · 2024-08-14T12:43:43Z

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 [email protected] list. Thank you for your contributions.

github-actions · 2024-10-14T12:46:09Z

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 [email protected] list. Thank you for your contributions.

github-actions · 2024-12-15T12:46:09Z

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 [email protected] list. Thank you for your contributions.

github-actions · 2024-12-23T12:45:56Z

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.

github-actions bot added the python label Aug 2, 2023

Polber force-pushed the jkinard/add-yaml-provider-tests branch from 6f5ba45 to 988146c Compare August 2, 2023 17:15

robertwb reviewed Aug 14, 2023

View reviewed changes

Polber force-pushed the jkinard/add-yaml-provider-tests branch from 988146c to 5070208 Compare September 12, 2023 23:51

Polber commented Sep 12, 2023

View reviewed changes

Polber requested a review from robertwb September 12, 2023 23:54

github-actions bot added the Next Action: Reviewers label Sep 13, 2023

[yaml] Add yaml_provider.py Unit Tests

ddf8b2a

Signed-off-by: Jeffrey Kinard <[email protected]>

Polber force-pushed the jkinard/add-yaml-provider-tests branch from 5070208 to 4477171 Compare September 13, 2023 18:15

address initial comments

6eea11a

Signed-off-by: Jeffrey Kinard <[email protected]>

Polber force-pushed the jkinard/add-yaml-provider-tests branch from 4477171 to 6eea11a Compare September 13, 2023 18:32

robertwb approved these changes Sep 18, 2023

View reviewed changes

github-actions bot added the slow-review label Sep 26, 2023

github-actions bot removed the slow-review label Sep 29, 2023

github-actions bot added the slow-review label Oct 7, 2023

AnandInguva added Next Action: Author and removed Next Action: Reviewers slow-review labels Oct 7, 2023

github-actions bot added the stale label Dec 6, 2023

github-actions bot removed the stale label Dec 7, 2023

github-actions bot added the stale label Feb 6, 2024

github-actions bot removed the stale label Feb 7, 2024

Polber marked this pull request as draft February 13, 2024 16:22

github-actions bot added the stale label Apr 14, 2024

github-actions bot removed the stale label Apr 15, 2024

github-actions bot added stale and removed stale labels Jun 14, 2024

github-actions bot added stale and removed stale labels Aug 14, 2024

github-actions bot added stale and removed stale labels Oct 14, 2024

github-actions bot added the stale label Dec 15, 2024

github-actions bot closed this Dec 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[yaml] Add yaml_provider.py Unit Tests #27804

[yaml] Add yaml_provider.py Unit Tests #27804

Polber commented Aug 2, 2023

codecov bot commented Aug 2, 2023 •

edited

Loading

github-actions bot commented Aug 2, 2023

Polber Sep 12, 2023

robertwb Sep 18, 2023

github-actions bot commented Sep 13, 2023

robertwb Sep 18, 2023

robertwb Sep 18, 2023

robertwb Sep 18, 2023

robertwb Sep 18, 2023

robertwb Sep 18, 2023

robertwb Sep 18, 2023

robertwb Sep 18, 2023

github-actions bot commented Sep 26, 2023

github-actions bot commented Sep 29, 2023

github-actions bot commented Oct 7, 2023

github-actions bot commented Dec 6, 2023

AnandInguva commented Dec 6, 2023

github-actions bot commented Feb 6, 2024

github-actions bot commented Apr 14, 2024

codecov-commenter commented Apr 14, 2024 •

edited by codecov bot

Loading

github-actions bot commented Jun 14, 2024

github-actions bot commented Aug 14, 2024

github-actions bot commented Oct 14, 2024

github-actions bot commented Dec 15, 2024

github-actions bot commented Dec 23, 2024

[yaml] Add yaml_provider.py Unit Tests #27804

[yaml] Add yaml_provider.py Unit Tests #27804

Conversation

Polber commented Aug 2, 2023

GitHub Actions Tests Status (on master branch)

codecov bot commented Aug 2, 2023 • edited Loading

Codecov Report

github-actions bot commented Aug 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 26, 2023

github-actions bot commented Sep 29, 2023

github-actions bot commented Oct 7, 2023

github-actions bot commented Dec 6, 2023

AnandInguva commented Dec 6, 2023

github-actions bot commented Feb 6, 2024

github-actions bot commented Apr 14, 2024

codecov-commenter commented Apr 14, 2024 • edited by codecov bot Loading

Codecov Report

github-actions bot commented Jun 14, 2024

github-actions bot commented Aug 14, 2024

github-actions bot commented Oct 14, 2024

github-actions bot commented Dec 15, 2024

github-actions bot commented Dec 23, 2024

codecov bot commented Aug 2, 2023 •

edited

Loading

codecov-commenter commented Apr 14, 2024 •

edited by codecov bot

Loading