search

container-storage-interface / spec

Container Storage Interface (CSI) Specification.
Apache License 2.0
1.34k stars 373 forks source link

Snapshot "Uploading" is too specific #299

Closed saad-ali closed 5 years ago

saad-ali commented 5 years ago

Multiple places in the snapshot API refer to "uploading" as an optional state of the snapshot. But "Uploading" is very specific and should be renamed to something more generic so that an SP is free to do any "post cut" "pre ready" work during this phase (not just uploading).

CC @xing-yang @jingxu97

saad-ali commented 5 years ago

uploading -> processing seems ok

xing-yang commented 5 years ago

This issue is related to https://github.com/container-storage-interface/spec/issues/302. We use "uploading" in many places because we have a "UPLOADING" enum state in SnapshotStatus. If we change "uploading" to "processing", we should also change "UPLOADING" to "PROCESSING" in SnapshotStatus. The proposal in #302 is to replace the enum states including "UPLOADING" with Boolean. Jing and I discussed about the possibility of using a Boolean READY state and get rid of all the enums. We need to decide what to do with #302 before fixing this issue.

saad-ali commented 5 years ago

More feedback from thockin: "Be careful with ready <- does it mean that it is ready to use or dependent on other states". Maybe try to be more precise: readyToUse?

jingxu97 commented 5 years ago

readyToUse seems good. But currently we are also use Ready boolean value in CSI VolumeSnapshot API object. Is that ok or we should also change to readyToUse there?

On Wed, Nov 7, 2018 at 8:49 AM Saad Ali notifications@github.com wrote:

More feedback from thockin: "Be careful with ready <- does it mean that it is ready to use or dependent on other states". Maybe try to be more precise: readyToUse?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/container-storage-interface/spec/issues/299#issuecomment-436693564, or mute the thread https://github.com/notifications/unsubscribe-auth/ASSNxfi8LWfbJjh804LNnChEDq4w9oZtks5usw8EgaJpZM4YRQCl .

--

jdef commented 5 years ago

if we're changing it, then we should be consistent

On Wed, Nov 7, 2018 at 1:38 PM Jing Xu notifications@github.com wrote:

readyToUse seems good. But currently we are also use Ready boolean value in CSI VolumeSnapshot API object. Is that ok or we should also change to readyToUse there?

On Wed, Nov 7, 2018 at 8:49 AM Saad Ali notifications@github.com wrote:

More feedback from thockin: "Be careful with ready <- does it mean that it is ready to use or dependent on other states". Maybe try to be more precise: readyToUse?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub < https://github.com/container-storage-interface/spec/issues/299#issuecomment-436693564 , or mute the thread < https://github.com/notifications/unsubscribe-auth/ASSNxfi8LWfbJjh804LNnChEDq4w9oZtks5usw8EgaJpZM4YRQCl

.

--

  • Jing

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/container-storage-interface/spec/issues/299#issuecomment-436731655, or mute the thread https://github.com/notifications/unsubscribe-auth/ACPVLCb3NHPPuvcmtHNgfmnlWso6UUUsks5usyi3gaJpZM4YRQCl .

saad-ali commented 5 years ago

Closed with https://github.com/container-storage-interface/spec/pull/307