borisdaeppen/vm-jiffybox

Perl API for JiffyBox

VM-JiffyBox

Perl API for JiffyBox

Development: https://github.com/borisdaeppen/VM-JiffyBox

Packaged: https://metacpan.org/module/VM::JiffyBox

vm-jiffybox's Issues

Naming schema for helper methods

get_id_from_name could be changed to get_id_for_name or get_id_by_name. This should then be the same for all other methods in the future who use the same kind of logic.

Bug: default value

make test error, works with:
has domain_name => (is => 'ro', default => sub {'https://api.jiffybox.de'});
has version => (is => 'ro', default => sub{'v1.0'});
Old Moo version?

$response->status_line doesn't cover all errors

This error is not returned

$VAR1 = bless( {
                 '_content' => 'LWP will support https URLs if the LWP::Protocol::https module
is installed.
',
                 '_rc' => 501,
                 '_headers' => bless( {
                                        'client-warning' => 'Internal response',
                                        'client-date' => 'Thu, 11 Jul 2013 18:57:39 GMT',
                                        'content-type' => 'text/plain',
                                        '::std_case' => {
                                                          'client-warning' => 'Client-Warning',
                                                          'client-date' => 'Client-Date'
                                                        }
                                      }, 'HTTP::Headers' ),
                 '_msg' => 'Protocol scheme \'https\' is not supported (LWP::Protocol::https not installed)',
                 '_request' => bless( {
                                        '_content' => '',
                                        '_uri' => bless( do{\(my $o = 'https://api.jiffybox.de/AUTH_KEY/v1.0/jiffyBoxes/BOX_ID')}, 'URI::https' ),
                                        '_headers' => bless( {
                                                               'user-agent' => 'libwww-perl/6.05'
                                                             }, 'HTTP::Headers' ),
                                        '_method' => 'GET'
                                      }, 'HTTP::Request' )
               }, 'HTTP::Response' );

Update Doc

This info is outdated: https://github.com/tim-schwarz/VM-JiffyBox

And also several typos of "Optinal"

enhance: create_vm

Parameters should be named.

The other API-features in this call should also be supported.
(Not only creation from backup-image)

get_id_from_name() doesn't work in perl v5.18.0

I can't get VM::JiffyBox working on a freshly installed Debian with perl v5.18.0. I'm still looking for the reason. Maybe it's the new perl-version...

add Perl::Critic as xt

add the gentle test as xt-Test

Doc Structure api <-> non-api

Maybe the new get_vms() should belong to chapter METHODS (SHORTCUTS) instead of METHODS (API) since it uses base_url . '/jiffyBoxes' like get_details() does.
It's actually a just shortcut to the existing subset offered.

But maybe the distinction between API and non-API calls doesn't make any sense... so I'll note it here.
Maybe there will be a decision once...

Box-ID required

must be checked in get_vm() before creating the box obj.

Tests Fail with Moo v1.000005

According to CPAN-Testers ( http://www.cpantesters.org/distro/V/VM-JiffyBox.html#VM-JiffyBox-0.002-TRIAL ) the tests fail with Moo v1.000005 as you can see in this report:

http://www.cpantesters.org/cpan/report/f51d7f8c-c8c0-11e2-a652-500531b64f85

I can reproduce the problem, when installing this version of Moo...

Bug in create_vm

Test 03 fails in testing parameter combination for create_vm ( related to #25 )

Check code in Perl::Critic

Use this service:

http://perlcritic.com

e.g. it reports this error for VM::JiffyBox::Box

Code before strictures are enabled at line 8, column 1.  See page 429 of PBP.  Severity: 5

Framework for Testing Results

We should think about, how we can test, if a method call (resulting in a HTTP-Request), ends in exactly this URL as expected. We can't test against the real JiffyBox-Provider in our "t"-directory!

Maybe it would be great if you could do:

my $hypervisor = VM::JiffyBox->new( token => 'bla', test_mode => 1 );

# now it gives back an URL instead of doing the actually request
my $URL = $hypervisor->do_bla();

# and we can test the URL in a t-script
ok ($URL, $excpected_urls{do_bla}, "check URL for do_bla()");

Good idea?
Same would be needed on Box-level... but the Box could look if it is set in the Hypervisor. No second implementation needed.

A very short c&p synopsis

add a very short copy and paste synopsis similar to a hello world

Depend on LWP::Protocol:https

We could make LWP::Protocol:https a dependency, since the JiffyBox-API needs this.

Add Usefull Onliners to the Docs

Like e.g.

perl -MVM::JiffyBox -e 'print VM::JiffyBox->new(token=>"API_TOKEN")->get_vm(BOX_ID)->get_details->{result}->{ips}->{public}->[0],"\n";'

for getting the IP

Test Module Creation

dito

Check for error messages in each call

Like this we could be more sure to get all the errors right...

 \ {
    messages   [
        [0] {
            message   "Ein unbekannter Fehler ist beim Ausfuehren des Kommandos aufgetreten.",
            type      "error"
        }
    ],
    result     JSON::XS::Boolean  {
        Parents       JSON::Boolean
        public methods (0)
        private methods (1) : __ANON__
        internals: 0
    }

Doc: Example with IP

bring this example to the docs:

my $ip    = $vm->get_information()->{result}->{ips}->{public}->[0];
my $ip    = $vm->get_ip();

Correct ERROR HANDLING in Docs

It says that All methods will return 0 on failure, but this is in fact no longer true. It just returns empty... (how ever this is called)

feature: is_ready()

shortcut to check if a machine state is ready...

or better: is_state('READY')?

Unclear Error Message on Failure in "get_id_from_name()"

This code

 56 sub get_id_from_name {
 57     my $self     = shift;
 58     my $box_name = shift || die 'box_name as argument needed';
 59 
 60     my $details = $self->get_details;
 61 
 62     $self->last         ( $details );
 63     $self->details_cache( $details );
 64 
 65     # look for a match in the results
 66     foreach my $box (values %{$details->{result}}) {
 67         return $box->{id} if ($box->{name} eq $box_name);
 68     }
 69 
 70     # if we reach here, this means there was no match
 71     return 0;
 72 }

shows this error

Can't use string ("0") as a HASH ref while "strict refs" in use at /home/testing/perl5/perlbrew/perls/perl-5.18.0/lib/site_perl/5.18.0/VM/JiffyBox.pm line 66.

if there is no result. Should be checked before loop.

Tests fail

Hallo Tim,

mach mal vom Hauptverzeichnis aus (im Branch "basic_structure") folgendes Kommando:

prove -Ilib t/

Dann kommt eine Fehlermeldung.
Kannst du die fixen?

Use a Version Flag for Protokol

like

$api_version = 'v1.0'

to check if client and server talk the same

add general method set_status()

start() and stop() are just special cases of the status-command. We should add a general method set_status() which would then be used like this

$vm->set_status('START') # or SHUTDOWN, PULLPLUG, FREEZE, THAW

Like this the user can use the whole variety of commands from the API.

(or we add separate methods for each process... like freeze())

Problem with Error-Handling

This way of error reporting is problematic:

    } else {
        return $details->status_line;
    }  

The user will not be able to know if his call was a success, because he will always get a result. For example, if I call an URL which doesn't exists, I get the following:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Frameset//EN"
    "http://www.w3.org/TR/xhtml1/DTD/xhtml1-frameset.dtd">
<html>
 <head>
    <title>T-Online Navigationshilfe</title>
 </head>
 <frameset rows="100%">
 <frame src="http://navigationshilfe.t-online.de/dnserror?url=http://blabla.diesnotexust.ch/"
    frameborder="0" noresize="noresize"/>
 <noframes>
  <body>
   <h1>Willkommen bei T-Online</h1>
   <p>
    <a href="http://navigationshilfe.t-online.de/dnserror?url=http://blabla.diesnotexust.ch/">
    weiter....</a>
   </p>
  </body>
 </noframes>
 </frameset>
</html>

So I have to check if the result is JSON or HTML, or what ever there will come. It is probably better to just return 0, '' or undef. Like this the user can just check the result with a simple if.
The status_line can still be stored in an object variable, so that the user can call for it if he wants to know why his request fails.

Nicer parsing of arguments in examples

my $auth_token = shift @ARGV || die 'Token as first argument needed!';

much nicer?

API-Errors

The API does not always tell its errors over the HTTP way, but inside the returned JSON. So we should maybe check the JSON for errors:

$VAR1 = {
          'messages' => [
                          {
                            'type' => 'error',
                            'message' => 'Der Name existiert bereits.'
                          }
                        ],
          'result' => bless( do{\(my $o = 0)}, 'JSON::XS::Boolean' )
        };

bug in example

change "answer" to "last"

UA as class or object variable?

Maybe this code in VM::JiffyBox::Box can be problematic:

my $ua = LWP::UserAgent->new();

I'm not very sure about the behaviour of this if a lot of objects interact...

It would be cleaner to implement the UserAgent as an object-variable to be sure that each object has it's own UserAgent. In this way UserAgents of different objects can not accidentally interact.

In classical OO-Perl this needs to be done in the constructor (in the new method), in Moo this can maybe done as a has variable (maybe with default)?

Projekt verschieben

Hallo Tim,

kannst du das Projket auf meinen Account verschieben?

Hier steht wie:

https://help.github.com/articles/how-to-transfer-a-repository

Renee und ich wollen etwas daran weiter arbeiten.

Danke und Gruss
Boris