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

Pass UrlFile object to Provider.checkFileUrl instead of string #1732

Conversation

MantisClone
Copy link

@MantisClone MantisClone commented Oct 7, 2022

Changes proposed in this PR:

  • Pass UrlFile object to Provider.checkFileUrl instead of a string

Depends on oceanprotocol/ocean.js#1627

Reasoning:

This is an incremental step towards supporting other file storage types like Arweave, Filecoin, etc...

@netlify
Copy link

netlify bot commented Oct 7, 2022

👷 Deploy request for market-oceanprotocol pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit d87b2e4

@vercel
Copy link

vercel bot commented Oct 7, 2022

@MantisClone is attempting to deploy a commit to the Ocean Protocol Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Oct 10, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
market Failed (Inspect) Oct 19, 2022 at 3:42PM (UTC)

@MantisClone
Copy link
Author

Hello, @kremalicious Thanks for taking a look at this and approving the CI checks to run. I believe the CI checks are failing because this PR depends on oceanprotocol/ocean.js#1627 which has not yet been merged/released.

@MantisClone
Copy link
Author

Hello @alexcos20 @kremalicious @mihaisc @bogdanfazakas

This PR depends on oceanprotocol/ocean.js#1627, but other than that, I think it is ready for review.

@kremalicious
Copy link
Contributor

kremalicious commented Oct 14, 2022

Misses one more usage of that file check, be sure to handle this for publish preview & published asset which is what the condition for fileInfoResponse in here is for:

// Get and set file info
useEffect(() => {
const oceanConfig = getOceanConfig(asset?.chainId)
if (!oceanConfig) return
async function initFileInfo() {
setFileIsLoading(true)
const providerUrl =
formikState?.values?.services[0].providerUrl.url ||
asset?.services[0]?.serviceEndpoint
try {
const fileInfoResponse = formikState?.values?.services?.[0].files?.[0]
.url
? await getFileUrlInfo(
formikState?.values?.services?.[0].files?.[0].url,
providerUrl
)
: await getFileDidInfo(asset?.id, asset?.services[0]?.id, providerUrl)
fileInfoResponse && setFileMetadata(fileInfoResponse[0])
// set the content type in the Dataset Schema
const datasetSchema = document.scripts?.namedItem('datasetSchema')
if (datasetSchema) {
const datasetSchemaJSON = JSON.parse(datasetSchema.innerText)
if (datasetSchemaJSON?.distribution[0]['@type'] === 'DataDownload') {
const contentType = fileInfoResponse[0]?.contentType
datasetSchemaJSON.distribution[0].encodingFormat = contentType
datasetSchema.innerText = JSON.stringify(datasetSchemaJSON)
}
}
setFileIsLoading(false)
} catch (error) {
LoggerInstance.error(error.message)
}
}
initFileInfo()
}, [asset, isMounted, newCancelToken, formikState?.values?.services])

@kremalicious
Copy link
Contributor

ocean.js v2.3.0 has the changes required for this PR

@MantisClone
Copy link
Author

Misses one more usage of that file check, be sure to handle this for publish preview & published asset which is what the condition for fileInfoResponse in here is for:

// Get and set file info
useEffect(() => {
const oceanConfig = getOceanConfig(asset?.chainId)
if (!oceanConfig) return
async function initFileInfo() {
setFileIsLoading(true)
const providerUrl =
formikState?.values?.services[0].providerUrl.url ||
asset?.services[0]?.serviceEndpoint
try {
const fileInfoResponse = formikState?.values?.services?.[0].files?.[0]
.url
? await getFileUrlInfo(
formikState?.values?.services?.[0].files?.[0].url,
providerUrl
)
: await getFileDidInfo(asset?.id, asset?.services[0]?.id, providerUrl)
fileInfoResponse && setFileMetadata(fileInfoResponse[0])
// set the content type in the Dataset Schema
const datasetSchema = document.scripts?.namedItem('datasetSchema')
if (datasetSchema) {
const datasetSchemaJSON = JSON.parse(datasetSchema.innerText)
if (datasetSchemaJSON?.distribution[0]['@type'] === 'DataDownload') {
const contentType = fileInfoResponse[0]?.contentType
datasetSchemaJSON.distribution[0].encodingFormat = contentType
datasetSchema.innerText = JSON.stringify(datasetSchemaJSON)
}
}
setFileIsLoading(false)
} catch (error) {
LoggerInstance.error(error.message)
}
}
initFileInfo()
}, [asset, isMounted, newCancelToken, formikState?.values?.services])

I looked at this briefly, but I don't know how to fix it.

@kremalicious
Copy link
Contributor

This needs merge or rebase against main and then continue. No need to update ocean.js in here then.

I looked at this briefly, but I don't know how to fix it.

ok, what did you try to solve this? What exactly is unclear?

@kremalicious kremalicious mentioned this pull request Nov 7, 2022
11 tasks
@kremalicious
Copy link
Contributor

closing as abandoned, and #1765 is picking things up from here.

@MantisClone MantisClone deleted the pass-url-object-instead-of-string branch November 14, 2022 18:40
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

2 participants