Re: [Fuego] [PATCH] Add support for get_board api on fserver

> -----Original Message-----
> From: Pavan Arun Deshpande <[email protected]>
> 
> Hi Tim,
> I have copied the fserver patch below.
> if  any suggestions let me know.
> 
> 
> Subject: [PATCH] Add support for get_board api on fserver
This should not be in the email message body. It duplicates the item
from the subject.

> 
> This allows a client to get board fields information
> from <board>.json file located on fserver
> 
> Signed-off-by: Pavan Arun Deshpande <[email protected] <mailto:[email protected]> >
> ---
>  fserver.py | 39 ++++++++++++++++++++++++++++++++++++---
>  1 file changed, 36 insertions(+), 3 deletions(-)
> 
> diff --git a/fserver.py b/fserver.py
> index a4bca89..e23d122 100755
> --- a/fserver.py
> +++ b/fserver.py
> @@ -64,11 +64,13 @@ VERSION=(0,6,0)
>  # 2. local fserver in Fuego container
>  # 3. test fserver on Tim's private server machine (birdcloud.org <http://birdcloud.org> )
>  # 4. test fserver on Tim's home desktop machine (timdesk)
> -base_dir = "/home/ubuntu/work/fserver/fserver-data"
> +#base_dir = "/home/ubuntu/work/fserver/fserver-data"
> +base_dir = "/home/pavan.ad/fserver/fserver/fserver-data <http://pavan.ad/fserver/fserver/fserver-data> "
>  if not os.path.exists(base_dir):
>      base_dir = "/usr/local/lib/fserver/fserver-data"
>  if not os.path.exists(base_dir):
> -    base_dir = "/home/tbird/work/fserver/fserver-data"
> +    #base_dir = "/home/tbird/work/fserver/fserver-data"
> +    base_dir = "/home/pavan.ad/fserver/fserver/fserver-data <http://pavan.ad/fserver/fserver/fserver-data> "

This is not the best way to add an additional auto-detected directory.
Don't remove or change any of the directories already checked by the code.
Instead, just rely on them not existing, and add the following two lines:

if not os.path.exists(base_dir):
    base_dir = "/home/pavan.ad/fserver/fserver-data"

It might be even better to auto-detect the base_dir, based on sys.argv[0].

I omitted this hunk from the patch that I applied.

> 
>  # this is used for debugging only
>  def log_this(msg):
> @@ -527,6 +529,37 @@ def do_update_board(req):
> 
>      send_response(result, msg)
> 
> +def do_get_board(req):
> +    req_data_dir = req.config.data_dir + os.sep + "boards"
> +    result = "OK"
> +    msg = ""
> +
> +    try:
> +        host = req.form["host"].value
> +        board = req.form["board"].value
> +    except:
> +        msg += "Error: missing host or board in form data"
> +        send_response("FAIL", msg)
> +        return
> +
> +    filename = "board-%s:%s" % (host, board)
> +    jfilepath = req_data_dir + os.sep + filename + ".json"
> +
> +    # check that board is already registered
> +    if not os.path.exists(jfilepath):
> +        msg += "Error: board '%s:%s' is not registered" % (host, board)
> +        send_response("FAIL", msg)
> +        return
> +
> +    #retrieving <board>.json fields into board dictionary
> +    import json
> +    board_fd = open(jfilepath, "r")
> +    msg = board_fd.read()
> +    board_fd.close()
> +
> +    send_response(result,msg)
> +
> +
>  def do_put_request(req):
>      req_data_dir = req.config.data_dir + os.sep + "requests"
>      result = "OK"
> @@ -1445,7 +1478,7 @@ def main(req):
>          log_this("DEBUG: in main(), after call to cgi.FieldStorage")
> 
>      action_list = ["show", "put_test", "put_run", "put_request",
> -            "put_binary_package", "put_board", "update_board",
> +            "put_binary_package", "put_board", "update_board", "get_board",
>              "query_boards", "query_requests", "query_runs", "query_tests",
>              "get_request", "get_run_url", "get_test",
>              "remove_request", "remove_test", "remove_run",
> --
> 2.17.1

The rest of this looks good.  Applied.

 -- Tim

P.S.  A couple more items of feeback.
1) private commentary that is not part of the commit message should be placed
   after the diffstat, after dashed line.

So this part:
> Hi Tim,
> I have copied the fserver patch below.
> if  any suggestions let me know.

Should have been put after the diffstat lines but before the first 'diff ... ' line

 2) it's better if you can get rid of the confidentiality notice.

> 
> Thanks and Regards
> Pavan Arun Deshpande
> 
> 
> 
> 
> This message contains confidential information and is intended only for the individual(s) named. If you are not the intended recipient, you
> are notified that disclosing, copying, distributing or taking any action in reliance on the contents of this mail and attached file/s is strictly
> prohibited. Please notify the sender immediately and delete this e-mail from your system. E-mail transmission cannot be guaranteed to be
> secured or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or contain viruses. The
> sender therefore does not accept liability for any errors or omissions in the contents of this message, which arise as a result of e-mail
> transmission.