Re: [Libguestfs] [PATCH v2v] -o rhv-upload: Give a nicer error if the storage domain does not exist

2023-01-27 Thread Nir Soffer 
On Fri, Jan 27, 2023 at 1:18 PM Nir Soffer  wrote:
>
> On Thu, Jan 26, 2023 at 2:31 PM Richard W.M. Jones  wrote:
> >
> > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1986386
> > Reported-by: Junqin Zhou
> > ---
> >  output/rhv-upload-precheck.py | 7 ++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/output/rhv-upload-precheck.py b/output/rhv-upload-precheck.py
> > index 1dc1b8498a..35ea021032 100644
> > --- a/output/rhv-upload-precheck.py
> > +++ b/output/rhv-upload-precheck.py
> > @@ -81,7 +81,12 @@ datacenter = data_centers[0]
> >
> >  # Get the storage domain.
> >  storage_domains = connection.follow_link(datacenter.storage_domains)
> > -storage_domain = [sd for sd in storage_domains if sd.name == 
> > params['output_storage']][0]
> > +try:
> > +storage_domain = [sd for sd in storage_domains \
> > +  if sd.name == params['output_storage']][0]
>
> Using `\` may work but it is needed. You can do this:
>
> storage_domain = [sd for sd in storage_domains
>if sd.name == params['output_storage']][0]
>
> This is also the common way to indent list comprehension that
> makes the expression more clear.
>
> > +except IndexError:
> > +raise RuntimeError("The storage domain ‘%s’ does not exist" %
> > +   params['output_storage'])
>
> The fix is safe and makes sense.
>
> Not sure why we list all storage domains when we already know the name,
> maybe Albert would like to clean up this mess later.

Like this:
https://github.com/oVirt/python-ovirt-engine-sdk4/blob/2aa50266056b7ee0b72597f346cbf0f006041566/examples/list_storage_domains.py#L93

___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH v2v] -o rhv-upload: Give a nicer error if the storage domain does not exist

2023-01-27 Thread Nir Soffer 
On Thu, Jan 26, 2023 at 2:31 PM Richard W.M. Jones  wrote:
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1986386
> Reported-by: Junqin Zhou
> ---
>  output/rhv-upload-precheck.py | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/output/rhv-upload-precheck.py b/output/rhv-upload-precheck.py
> index 1dc1b8498a..35ea021032 100644
> --- a/output/rhv-upload-precheck.py
> +++ b/output/rhv-upload-precheck.py
> @@ -81,7 +81,12 @@ datacenter = data_centers[0]
>
>  # Get the storage domain.
>  storage_domains = connection.follow_link(datacenter.storage_domains)
> -storage_domain = [sd for sd in storage_domains if sd.name == 
> params['output_storage']][0]
> +try:
> +storage_domain = [sd for sd in storage_domains \
> +  if sd.name == params['output_storage']][0]

Using `\` may work but it is needed. You can do this:

storage_domain = [sd for sd in storage_domains
   if sd.name == params['output_storage']][0]

This is also the common way to indent list comprehension that
makes the expression more clear.

> +except IndexError:
> +raise RuntimeError("The storage domain ‘%s’ does not exist" %
> +   params['output_storage'])

The fix is safe and makes sense.

Not sure why we list all storage domains when we already know the name,
maybe Albert would like to clean up this mess later.

Nir

___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH v2v] -o rhv-upload: Give a nicer error if the storage domain does not exist

2023-01-26 Thread Richard W.M. Jones 
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1986386
Reported-by: Junqin Zhou
---
 output/rhv-upload-precheck.py | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/output/rhv-upload-precheck.py b/output/rhv-upload-precheck.py
index 1dc1b8498a..35ea021032 100644
--- a/output/rhv-upload-precheck.py
+++ b/output/rhv-upload-precheck.py
@@ -81,7 +81,12 @@ datacenter = data_centers[0]
 
 # Get the storage domain.
 storage_domains = connection.follow_link(datacenter.storage_domains)
-storage_domain = [sd for sd in storage_domains if sd.name == 
params['output_storage']][0]
+try:
+storage_domain = [sd for sd in storage_domains \
+  if sd.name == params['output_storage']][0]
+except IndexError:
+raise RuntimeError("The storage domain ‘%s’ does not exist" %
+   params['output_storage'])
 
 # Get the cluster.
 clusters = connection.follow_link(datacenter.clusters)
-- 
2.39.0

___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs