search

Joystream / joystream

Joystream Monorepo
http://www.joystream.org
GNU General Public License v3.0
1.42k stars 115 forks source link

colossus: do not serve content from pending folder #4989

Closed mnaamani closed 11 months ago

mnaamani commented 11 months ago

It is not clearly obvious how the node will behave, if while service a file from temp pending, the file is moved/renamed into the uploads folder. Will the move fail or will the http request handler to getFile fail. So it seems safer and more predictable behavior to only serve objects in the main uploads folder.

This PR reverts change added in https://github.com/Joystream/joystream/pull/4971 which allows serving objects in pending folder.

mnaamani commented 11 months ago

Looks good to me, the only implication of this change is that even the uploader won't able to access the file till it's has been accepted.

Yes that is unfortunate, best case scenario is they would have to wait up to 6s for file to become accessible.

The other possible solution (and might not be trivial to implement) is that whenever the conflict happens we let the HTTP request fail and the move should always succeed. WDYT?

Yes I can think of a few ways, but it does seem complex. The root issue is of course having separate location for pending folder. Would it be less complex if we eliminated the pending folder, and track the objects that need to be "confirmed" with onchain tx in a local db, or simply a JSON file?

zeeshanakram3 commented 11 months ago

The root issue is of course having separate location for pending folder. Would it be less complex if we eliminated the pending folder, and track the objects that need to be "confirmed" with onchain tx in a local db, or simply a JSON file?

Sounds good. Maybe this can be implemented as a feature in storage-squid (i.e. as a mutation that will store relevant info in storage-squid db)

mnaamani commented 11 months ago

Sounds good. Maybe this can be implemented as a feature in storage-squid (i.e. as a mutation that will store relevant info in storage-squid db)

Worth considering.