Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore 404 errors when downloading assets #19

Merged
merged 3 commits into from
Dec 19, 2024

Conversation

atombender
Copy link
Member

@atombender atombender commented Dec 19, 2024

This currently causes the export to fail. I have not looked into why assets would be gone in this case, but that seems like a separate issue that is probably caused by asset documents getting out of async with the files, rather than stuff genuinely going missing. Either way, it should not prevent an export from completing.

Note that we don't have a great way to warn the user, as far as I can see. The importer has a way to collect warnings, but threading a similar mechanism through the exporter would be a bit of a chore, so I didn't do that yet.

@atombender atombender requested a review from a team December 19, 2024 12:06
@@ -112,6 +112,12 @@ class AssetHandler {
try {
return await this.downloadAsset(assetDoc, dstPath)
} catch (err) {
// Ignore missing assets
if (err.statusCode === 404) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest that we also not fail on 401/403. Similarly, the dataset is somehow odd/a bit broken, but we can still export as much as is accessible to the user.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point! Updated.

@atombender atombender merged commit 8f5df3d into main Dec 19, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants