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

Add Batch CID route to creator node #1324

Merged
merged 26 commits into from
Mar 19, 2021
Merged

Conversation

cheran-senthil
Copy link
Contributor

Description

See #1310 for details

@cheran-senthil cheran-senthil added the content-node Content Node (previously known as Creator Node) label Mar 19, 2021
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.

Looking pretty good so far, some minor comments

creator-node/src/routes/files.js Outdated Show resolved Hide resolved
creator-node/src/routes/files.js Show resolved Hide resolved
creator-node/src/routes/files.js Outdated Show resolved Hide resolved
* check if cids exists
* Use let instead of const
* Use raw: true to make it more performant
Copy link
Contributor

@SidSethi SidSethi left a comment

Choose a reason for hiding this comment

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

looks great - couple nits then re-request

also don't forget to comment+close the old stale PRs, also would prefer you just copy/paste the PR description into this one rather than linking out

creator-node/docker-compose/development.env Outdated Show resolved Hide resolved
creator-node/default-config.json Outdated Show resolved Hide resolved
creator-node/src/routes/files.js Outdated Show resolved Hide resolved
creator-node/src/routes/files.js Outdated Show resolved Hide resolved
SidSethi
SidSethi previously approved these changes Mar 19, 2021
Copy link
Contributor

@SidSethi SidSethi left a comment

Choose a reason for hiding this comment

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

great work!

@SidSethi
Copy link
Contributor

also - make sure CI passes before merging (not a blocker for hotfixing to staging)

@cheran-senthil
Copy link
Contributor Author

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.

Great work

@cheran-senthil
Copy link
Contributor Author

@cheran-senthil cheran-senthil merged commit c120330 into master Mar 19, 2021
@cheran-senthil cheran-senthil deleted the vss-hotfix-batch-cids branch March 19, 2021 20:50
@cheran-senthil
Copy link
Contributor Author

cheran-senthil commented Mar 20, 2021

Forgot two brackets, this endpoint is borked, should be

({ storagePath }) => fs.pathExists(storagePath)

instead of,

( storagePath ) => fs.pathExists(storagePath)

Made changes on: #1310

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content-node Content Node (previously known as Creator Node)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants