Rblp / Rblpapi

R package interfacing the Bloomberg API from https://www.bloomberglabs.com/api/
Other
167 stars 75 forks source link

check if user define remote resources #360

Closed creamy-gurkin closed 2 years ago

creamy-gurkin commented 2 years ago

Hi, I'm not sure if this is the correct branch to merge to, but I wanted to use the library on a machine not directly connected to the internet and didn't see a variable to update for that use case so I added one. I'm also unsure about where to update the documentation on this variable, but if you let me know then I wouldn't mind updating that as well as well as update anything else you think I might have missed.

Many thanks.

eddelbuettel commented 2 years ago

That is an extension we could support. And your code is straighgforward (though the long code on one line is ... non standard).

But consider this alternative form:

#!/bin/sh

: ${MYVAL="myvalueis42"}
echo "MYVAL is ${MYVAL}"

otherval="something here"
if [ -z ${OTHERVAL+otherval} ]; then
    OTHERVAL=$otherval
    echo "using ${OTHERVAL}"
else
    echo "using user defined ${OTHERVAL}"
fi

Save it as, say, /tmp/snippet and make it mode 0755.

edd@rob:~$ /tmp/snippet.sh 
MYVAL is myvalueis42
using something here
edd@rob:~$ MYVAL="blue house" /tmp/snippet.sh 
MYVAL is blue house
using something here
edd@rob:~$ OTHERVAL="orange guppy" MYVAL="blue house" /tmp/snippet.sh 
MYVAL is blue house
using user defined orange guppy
edd@rob:~$ 

In short, we only need the initial form : ${VALUE="default"} but could/should maybe test for a resource to be present (but then it could be a remote URL like the default value so a little hard to test -f.

Would you mind simplifying your PR?

johnlaing commented 2 years ago

Could you share a bit more about your use case? The package does require a connection to Bloomberg to be usable, so I'm wondering if you might be heading down a dead end by installing on a machine with no internet.

creamy-gurkin commented 2 years ago

@johnlaing my use case is being behind a corporate firewall and although we do have a connection to Bloomberg, there are various reasons for me not to want to open a connection to github. This way we can load the resources locally and build the package with little issue. I see this as a small and isolated adjustment to the library and a fine feature for anyone in my position. :-)

@eddelbuettel no problem. I assume you'll want to maintain control of the naming of the variables in the main configure so I just chose some generic names for the snippet but since it isn't a function with return you'll have to call it once per download block. Just let me know if anything else needs changing. Also I used the -z test since I figured knowing whether or not val was an empty string might be useful.

eddelbuettel commented 2 years ago

@creamy-gurkin I think you misunderstood 100%. I didn't ask you to add /tmp/snippet.sh. I showed you in that file what changes I was expecting you to make to configure.

It would also be nice you could log the change to configure in the file ChangeLog with a name and email address.

eddelbuettel commented 2 years ago

I am going to close this and replace it with quick and simple new PR. "Works here" as they say and will give it some more testing paces.

In this example I set both blpLibrary and blpHeaders to local files, the configure echos in two lines starting with ** using ... that it deploys them. If unset, downloads will be made as usual.

edd@rob:~/git/rblpapi(feature/local_archive_option)$ blpLibrary=/tmp/demo/blpLibrary.tar.gz blpHeaders=/tmp/demo/blpHeaders.tar.gz R CMD INSTALL .
* installing to library ‘/usr/local/lib/R/site-library’                                                                                                                                                            
* installing *source* package ‘Rblpapi’ ...
** using non-staged installation via StagedInstall field                                                                                                                                                           
Setting up compilation for a linux 64-bit system
** using /tmp/demo/blpHeaders.tar.gz
** using /tmp/demo/blpLibrary.tar.gz
** libs
[.... rest of compilation omitted ... ]
eddelbuettel commented 2 years ago

Replaced by #362

creamy-gurkin commented 2 years ago

Sure, sorry I was less responsive. My thanks for the update.