You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "shunping (via GitHub)" <gi...@apache.org> on 2024/09/20 17:15:17 UTC
[PR] Fix internal unit test failure [beam]
shunping opened a new pull request, #32518:
URL: https://github.com/apache/beam/pull/32518
Skip or mock the check of is_soft_delete_enabled, because it requires a networck connection to access GCS.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GCSIO] Fix internal unit test failure [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on code in PR #32518:
URL: https://github.com/apache/beam/pull/32518#discussion_r1769353124
##########
sdks/python/apache_beam/options/pipeline_options_test.py:
##########
@@ -718,6 +719,7 @@ def test_validation_good_stg_good_temp(self):
'--staging_location=gs://beam/stg',
'--temp_location=gs://beam/tmp'
])
+ options.view_as(TestOptions).dry_run = True
Review Comment:
I tend to prefer mock as dryRun is too broad and may affect other behaviors that reduce test coverage
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GCSIO] Fix internal unit test failure [beam]
Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on PR #32518:
URL: https://github.com/apache/beam/pull/32518#issuecomment-2364641165
@Abacn could you take an other look?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GCSIO] Fix internal unit test failure [beam]
Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on code in PR #32518:
URL: https://github.com/apache/beam/pull/32518#discussion_r1769367730
##########
sdks/python/apache_beam/options/pipeline_options_test.py:
##########
@@ -718,6 +719,7 @@ def test_validation_good_stg_good_temp(self):
'--staging_location=gs://beam/stg',
'--temp_location=gs://beam/tmp'
])
+ options.view_as(TestOptions).dry_run = True
Review Comment:
An example of test failure if mocking is used https://github.com/apache/beam/actions/runs/10963819700/job/30446103406
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix internal unit test failure [beam]
Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on code in PR #32518:
URL: https://github.com/apache/beam/pull/32518#discussion_r1768972414
##########
sdks/python/apache_beam/options/pipeline_options.py:
##########
@@ -1023,7 +1023,8 @@ def _handle_temp_and_staging_locations(self, validator):
def validate(self, validator):
errors = []
if validator.is_service_runner():
- errors.extend(self._handle_temp_and_staging_locations(validator))
+ if not self.view_as(TestOptions).dry_run:
Review Comment:
Done.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix internal unit test failure [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #32518:
URL: https://github.com/apache/beam/pull/32518#issuecomment-2364173010
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment `assign set of reviewers`
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix internal unit test failure [beam]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #32518:
URL: https://github.com/apache/beam/pull/32518#issuecomment-2364285242
## [Codecov](https://app.codecov.io/gh/apache/beam/pull/32518?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 57.36%. Comparing base [(`75a4637`)](https://app.codecov.io/gh/apache/beam/commit/75a463790d7912c475939f8c63d62ffba2002b0a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`a508281`)](https://app.codecov.io/gh/apache/beam/commit/a5082814f17704250080dfa4d4d98825ba02aad8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 3 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #32518 +/- ##
============================================
- Coverage 57.37% 57.36% -0.01%
Complexity 1474 1474
============================================
Files 966 966
Lines 153593 153595 +2
Branches 1076 1076
============================================
- Hits 88118 88114 -4
- Misses 63271 63277 +6
Partials 2204 2204
```
| [Flag](https://app.codecov.io/gh/apache/beam/pull/32518/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [python](https://app.codecov.io/gh/apache/beam/pull/32518/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.56% <100.00%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/beam/pull/32518?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GCSIO] Fix internal unit test failure [beam]
Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on code in PR #32518:
URL: https://github.com/apache/beam/pull/32518#discussion_r1769369719
##########
sdks/python/apache_beam/options/pipeline_options_test.py:
##########
@@ -718,6 +719,7 @@ def test_validation_good_stg_good_temp(self):
'--staging_location=gs://beam/stg',
'--temp_location=gs://beam/tmp'
])
+ options.view_as(TestOptions).dry_run = True
Review Comment:
Actually, I have some idea which may need more changes. Will try and report back.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix internal unit test failure [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on code in PR #32518:
URL: https://github.com/apache/beam/pull/32518#discussion_r1768951994
##########
sdks/python/apache_beam/options/pipeline_options.py:
##########
@@ -1023,7 +1023,8 @@ def _handle_temp_and_staging_locations(self, validator):
def validate(self, validator):
errors = []
if validator.is_service_runner():
- errors.extend(self._handle_temp_and_staging_locations(validator))
+ if not self.view_as(TestOptions).dry_run:
Review Comment:
Let's add a comment stating "this needs Internet connection and/or credentials"
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix internal unit test failure [beam]
Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on PR #32518:
URL: https://github.com/apache/beam/pull/32518#issuecomment-2364171344
r: @ahmedabu98 @Abacn
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GCSIO] Fix internal unit test failure [beam]
Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on code in PR #32518:
URL: https://github.com/apache/beam/pull/32518#discussion_r1769357960
##########
sdks/python/apache_beam/options/pipeline_options_test.py:
##########
@@ -718,6 +719,7 @@ def test_validation_good_stg_good_temp(self):
'--staging_location=gs://beam/stg',
'--temp_location=gs://beam/tmp'
])
+ options.view_as(TestOptions).dry_run = True
Review Comment:
Unfortunately, mock does not work when tests are run in our GitHub actions. Because we don't have google-cloud-storage installed for these unit tests on pipeline. As a result, mocking on 'apache_beam.io.gcp.gcsio' will give an error, and I don't want to have a condition inside the test to say "if we can load the package, we will do the mock; otherwise we skip it".
I am open to any suggestions though.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GCSIO] Fix internal unit test failure [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn merged PR #32518:
URL: https://github.com/apache/beam/pull/32518
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GCSIO] Fix internal unit test failure [beam]
Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on code in PR #32518:
URL: https://github.com/apache/beam/pull/32518#discussion_r1769533928
##########
sdks/python/apache_beam/options/pipeline_options_test.py:
##########
@@ -718,6 +719,7 @@ def test_validation_good_stg_good_temp(self):
'--staging_location=gs://beam/stg',
'--temp_location=gs://beam/tmp'
])
+ options.view_as(TestOptions).dry_run = True
Review Comment:
Ok. I made some changes to mock conditionally. PTAL
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GCSIO] Fix internal unit test failure [beam]
Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on code in PR #32518:
URL: https://github.com/apache/beam/pull/32518#discussion_r1769533928
##########
sdks/python/apache_beam/options/pipeline_options_test.py:
##########
@@ -718,6 +719,7 @@ def test_validation_good_stg_good_temp(self):
'--staging_location=gs://beam/stg',
'--temp_location=gs://beam/tmp'
])
+ options.view_as(TestOptions).dry_run = True
Review Comment:
Ok. I made some changes. PTAL
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org