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

Fix discprov image dirCID indexing #280

Merged
merged 3 commits into from
Feb 6, 2020

Conversation

SidSethi
Copy link
Contributor

@SidSethi SidSethi commented Feb 4, 2020

Fixes discprov image indexing when local cat fails. Regression occurred after we added multi-res images and changed the cnode ipfs gateway route API. The indexing code was still pinging the non-dir image gateway route on cnode, which no longer returns error msg indicating CID is a directory. Logic now first checks the dir image gateway route.

Tested fallback logic by removing the local cat logic at beginning of function so it has to ping the gateways.
Tested non-dir image fallback logic by running local discprov against staging chain from scratch.

@SidSethi
Copy link
Contributor Author

SidSethi commented Feb 4, 2020

Copy link
Contributor

@hareeshnagaraj hareeshnagaraj left a comment

Choose a reason for hiding this comment

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

looks good to me, i would prefer if we could somehow emulate the single byte cat (maybe with request streaming up to a limited size or something - which is possible in python) but let's get this in, we will have to revisit our dir behavior eventually anyway.

let's confirm this works by running locally against stage before checking in?

Copy link
Contributor

@dmanjunath dmanjunath left a comment

Choose a reason for hiding this comment

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

Overall looks good! some small comments

discovery-provider/src/utils/ipfs_lib.py Outdated Show resolved Hide resolved
discovery-provider/src/utils/ipfs_lib.py Outdated Show resolved Hide resolved
discovery-provider/src/utils/ipfs_lib.py Outdated Show resolved Hide resolved
discovery-provider/src/utils/ipfs_lib.py Show resolved Hide resolved
@SidSethi SidSethi force-pushed the ss-discprov-dircid-indexing-bugfix branch from 9ec8d37 to f0b555f Compare February 5, 2020 21:53
@SidSethi SidSethi force-pushed the ss-discprov-dircid-indexing-bugfix branch from f0b555f to ee7af53 Compare February 6, 2020 01:52
@SidSethi SidSethi merged commit 444b415 into master Feb 6, 2020
@SidSethi SidSethi deleted the ss-discprov-dircid-indexing-bugfix branch February 6, 2020 17:43
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.

None yet

3 participants