[Kimchi-devel] [PATCH] issue #447: Check download URL prior to start Task
Aline Manera
alinefm at linux.vnet.ibm.com
Thu Sep 25 15:03:04 UTC 2014
On 09/25/2014 11:43 AM, Crístian Viana wrote:
> On 24-09-2014 23:08, Aline Manera wrote:
>> + # Verify if the URL is valid
>> + if create_param == 'url':
>> + url = params['url']
>> + try:
>> + urllib2.urlopen(url)
>
> We should validate the specific parameters for the 'url' case inside
> the function '_create_volume_with_url', just like we check if the
> upload file exists before reading it inside the function
> '_create_volume_with_file'.
I have done it in my first implementation but it does not work well on
UI perspective.
Let me explain.
If we do this verification on _create_volume_with_url means the POST
/storagepools/<pool>/storagevolumes will return 202 (success)
On UI side, the "Add volume" dialog will be closed and a new "volume" is
displayed in the pool indicating it is being downloaded.
But as far as the new volume is added, it appears as "Failed" as the
Task finished with a failed status as the URL is not valid.
Doing the verification prior to start the Task means POST
/storagepools/<pool>/storagevolumes will return 500 (failed)
On UI side, the "Add volume" dialog will keep open, so user can modify
the URL and only see a "in progress volume" when it is really in progress.
>
> Actually, I think the best way to fix this problem is by enlarging the
> try/except block to include the existing urllib2.urlopen call. We
> don't need to make two requests. That call will raise the same
> exception there. The problem now is that it's not included inside the
> try/except block, hence the "Unexpected exception".
>
> But if we do keep this approach, you still need to close the return
> variable of "urllib2.urlopen".
Good point. I will do that on V2.
More information about the Kimchi-devel
mailing list