Summary

Improve fetching of original/patched file content.

Review Request #12075 — Created Feb. 23, 2022 and submitted Feb. 25, 2022, 7:07 p.m.

Information

Owner

chipx86

Repository

ReviewBot

Branch

release-3.0.x

Bugs

Depends On

Reviewers

Groups

reviewbot

People

Description

We no longer attempt to fetch original content for new files or patched

content for deleted files. Instead, these are immediately returned as

None.
File content fetching is also better protected, handling HTTP 404 and

500 results, which both will return None. Previously, HTTP 404 results

were handled when fetching file paths, but not file content, which was

backwards. HTTP 500 wasn't handled at all, instead causing crashes.

Testing Done

Relevant unit tests pass.

Commits

Summary	ID
Improve fetching of original/patched file content. We no longer attempt to fetch original content for new files or patched content for deleted files. Instead, these are immediately returned as `None`. File content fetching is also better protected, handling HTTP 404 and 500 results, which both will return `None`. Previously, HTTP 404 results were handled when fetching file paths, but not file content, which was backwards. HTTP 500 wasn't handled at all, instead causing crashes.	eaef6efa6c25c66893bbf5e16311f057978c4de5

Summary

Improve fetching of original/patched file content.

We no longer attempt to fetch original content for new files or patched content for deleted files. Instead, these are immediately returned as `None`. File content fetching is also better protected, handling HTTP 404 and 500 results, which both will return `None`. Previously, HTTP 404 results were handled when fetching file paths, but not file content, which was backwards. HTTP 500 wasn't handled at all, instead causing crashes.

eaef6efa6c25c66893bbf5e16311f057978c4de5

Issues

Description	From	Last Updated
F401 'reviewbot.processing.review.File' imported but unused	reviewbot	Feb. 23, 2022, 8:11 p.m.
F811 redefinition of unused 'test_original_file_contents_with_http_404' from line 469	reviewbot	Feb. 23, 2022, 8:11 p.m.
F811 redefinition of unused 'test_patched_file_contents_with_http_404' from line 544	reviewbot	Feb. 23, 2022, 8:11 p.m.

flake8 failed.

JSHint passed.

flake8

bot/reviewbot/processing/tests/test_file.py (Diff revision 1)
The issue has been resolved. Show all issues
```
F401 'reviewbot.processing.review.File' imported but unused
```
bot/reviewbot/processing/tests/test_file.py (Diff revision 1)
The issue has been resolved. Show all issues
```
F811 redefinition of unused 'test_original_file_contents_with_http_404' from line 469
```
bot/reviewbot/processing/tests/test_file.py (Diff revision 1)
The issue has been resolved. Show all issues
```
F811 redefinition of unused 'test_patched_file_contents_with_http_404' from line 544
```

Change Summary:


Fixed duplicate test names.
Removed an unused import.

Commits:

	Summary	ID
	Improve fetching of original/patched file content. We no longer attempt to fetch original content for new files or patched content for deleted files. Instead, these are immediately returned as `None`. File content fetching is also better protected, handling HTTP 404 and 500 results, which both will return `None`. Previously, HTTP 404 results were handled when fetching file paths, but not file content, which was backwards. HTTP 500 wasn't handled at all, instead causing crashes.	2362cf581e8ac276fe447da6d1fa5fc5e4b94419
	Improve fetching of original/patched file content. We no longer attempt to fetch original content for new files or patched content for deleted files. Instead, these are immediately returned as `None`. File content fetching is also better protected, handling HTTP 404 and 500 results, which both will return `None`. Previously, HTTP 404 results were handled when fetching file paths, but not file content, which was backwards. HTTP 500 wasn't handled at all, instead causing crashes.	eaef6efa6c25c66893bbf5e16311f057978c4de5

Diff:

Revision 2 (+816 -90)

Show changes

	bot/reviewbot/processing/review.py
	bot/reviewbot/processing/tests/test_file.py
	bot/reviewbot/testing/testcases.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (1f919bf)