Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default blpAuthenticate object support redux #285

Merged
merged 22 commits into from
Oct 29, 2019
Merged

Conversation

randomee
Copy link
Contributor

@randomee randomee commented Jan 5, 2019

cleanup of #283

Add support for default authentication object (modeled after support for default connection obj)

- one new function - defaultAuthentication()
  more or less "sed s/Connection/Authentication/g < defaultConnection.R > defaultAuthentication.R"
- export defaultAuthentication() to the NAMESPACE
- change default args for "data" functions to use defaultAuthentication() not NULL
- setup support for "blpAutoAuthenticate" akin to "blpAutoConnect".  Again, more or less copy/paste
- documentation for default auth in the intro vignette

…for default connection obj)

    - one new function - defaultAuthentication()
      more or less "sed s/Connection/Authentication/g < defaultConnection.R > defaultAuthentication.R"
    - export defaultAuthentication() to the NAMESPACE
    - change default args for "data" functions to use defaultAuthentication() not NULL
    - setup support for "blpAutoAuthenticate" akin to "blpAutoConnect".  Again, more or less copy/paste
    - documentation for default auth in the intro vignette
@randomee
Copy link
Contributor Author

randomee commented Jan 5, 2019

Addresses @johnlaing 's note about saving a default auth object they way Rbbg did/does. #251

revert localhost host changes
randomee added 2 commits May 20, 2019 02:04
   work correctly
- tested "application name" authentication with sapi
- updated documentation to note sapi can use app name authentication as
   well
@johnlaing
Copy link
Contributor

My bad for dropping the ball on this a few months back, but thank you for following up. Here are some first-pass comments:

  • I wasn't aware that SAPI with application ID was an option. Is that documented somewhere?

  • We need to sort out the IP address resolution from host. The test is.null(ip.address) will now always be false because a default value is provided. Yet the default value of localhost will not work, as this is not an IP address.

  • The blpAuthenticate function must continue to return its identity, even in the default=TRUE case. Existing code relied on getting an identity back without having to specify default=FALSE; we can't break that. We could return it invisibly, however.

  • The init workflow needs to be tweaked. What happens if we try to auto-authenticate without having already connected? I'm sure this will fail, but I'm not quite sure where and whether it will be easy to diagnose.

@randomee
Copy link
Contributor Author

No worries. We're all busy. And I'm just scratching my own itch.

I never looked for application-id auth over SAPI in the docs. BBG support said to use it, and, well, it seems to work. (Thank you @alfredkanzler for getting it working before I knew it was an option or needed!)

IP address, I'll look at.

I think blpAuthenticate is behaving in a way that will continue to work. Is this what you mean?

> testIP <- "x.x.x.x"
> testUUID <- "xxxxxxxx"
> testSAPI <- "y.y.y.y"
>
> library(Rblpapi, quietly=TRUE)
Rblpapi version 0.3.10 using Blpapi headers 3.8.18.1 and run-time 3.8.18.1.
Please respect the Bloomberg licensing agreement and terms of service.
> blpConnect(host=testSAPI)
> blpid<-blpAuthenticate(ip=testIP, uuid=testUUID)
> bdp("IBM US Equity", "NAME", identity=blpid)
                                     NAME
IBM US Equity INTL BUSINESS MACHINES CORP
> bdp("IBM US Equity", "NAME")
                                     NAME
IBM US Equity INTL BUSINESS MACHINES CORP
> blpid
<pointer: 0x55f1c411cbf0>
>

The error for no-blpAutoConnect but with blpAutoAuthenticate seems like it has enough info to point users the right way. Is this enough, or should it be more explicit?

> testIP <- "x.x.x.x"
> testUUID <- "xxxxxxxx"
> testSAPI <- "y.y.y.y"
> options("uuid" = testUUID)
> options("blpIP" = testIP)
> options("blpHost" = testSAPI)
> options("blpAutoAuthenticate" = TRUE)
> library(Rblpapi, quietly=TRUE)
Rblpapi version 0.3.10 using Blpapi headers 3.8.18.1 and run-time 3.8.18.1.
Please respect the Bloomberg licensing agreement and terms of service.
Error: package or namespace load failed forRblpapi:
 .onAttach failed in attachNamespace() for 'Rblpapi', details:
  call: NULL
  error: No connection object has been created. Use 'blpConnect()' first.
>

@randomee
Copy link
Contributor Author

Would changing the localhost code from is.null() to a compare be reasonable?

if (is.null(ip.address)) {

changed to

if (ip.address == "localhost") {

None of the setups I have access to run SAPI on localhost, so I can't really test it fully. I can make the commit for someone else to verify though.

@johnlaing
Copy link
Contributor

You are right - it does still work. I had anticipated that it might not return the connection object if default=FALSE but it does. I think we could make that a little more clear, however.

I think you may be confusing the host argument between blpConnect and blpAuthenticate. In the former it refers to the SAPI server, but in the latter it refers to whatever machine you are logged in from. In fact, the authentication service wants an IP address which can be passed explicitly via ip.address. We also provide the host argument as a convenience. (My workstation name never changes, but its local IP might from time to time.)

I think the test is.null(ip.address) is good, we just have to change the default argument to ip.address=getOption("blpIP", NULL)

@randomee
Copy link
Contributor Author

randomee commented May 23, 2019

Ah, ok. I never groked that the hostname here is not the same as for blpConnect. I don't think the documentation on that is clear at all. In that case, perhaps there should be an error/warning thrown if both IP and host are passed in. Silently taking the IP, when the hostname may resolve differently, seems like a potentially really annoying problem to debug.

That also means host=getOption("blpHost", "localhost") is incorrect. And there should be a separate default options() variable. Maybe blpLoginHostname? kinda verbose, but clearer.

Changed, and pushed the NULL default. I have not had a chance to test it, yet. I'll update when I have. It still needs documentation and code changes for the above discussion points.

randomee added 3 commits May 23, 2019 15:54
- change blpIP default to NULL, so we can fall through to hostname lookup
- change options() name for host to make it clearer that the hostname is
  where the uuid/user last authenticated.  Not the same hostname as in
  blpConnect.
@eddelbuettel
Copy link
Member

This seems to move nicely in the right direction as everybody can still run it. @randomee an entry in file ChangeLog would be appreciated too. (Uses an old GNU/FSF format that Emacs happens to make easy; two spaces around name and email. Of course you can mock it by hand in another editor ....)

@randomee
Copy link
Contributor Author

  • Cleaned up documentation
  • added warning if both ip.address and host are passed to blpAuthenticate()
  • renamed the options() variable names to make the host vs host distinction clearer
  • renamed options(uuid) to blpUUID to keep the namespace more consistient

I'll test this tomorrow on a variety of machines and confirm that it is working where I can test it.

fix test logic
@randomee
Copy link
Contributor Author

Looks reasonable and working for a good sample set. All SAPI & DAPI calls are against one company's infra. I'll test 2 more tomorrow. If there are errors, I'll comment.

If there are more code paths to test, let me know and I'll see what I can do.

I cannot test against B-PIPE, or SAPI running on localhost. If someone could check those, that would be nice.

Otherwise, I think it's in pretty good shape. It would be nice to run roxygen2::roxygenise() to update the various docs. I can do that, or leave it for the next release cycle process.

With SAPI, uuid, manual connect, & auth
Note warning for supplying both IP & hostname.

> testIP <- "x.x.x.x"
> testUUID <- "yyyyyyy"
> testSAPI <- "z.z.z.z"
> options("blpUUID" = testUUID)
> options("blpLoginIP" = testIP)
> options("blpHost" = testSAPI)
> options("blpPort" = 8194)
> options("blpLoginHostname" = testIP)
> library(Rblpapi, quietly=TRUE)
Rblpapi version 0.3.10 using Blpapi headers 3.8.18.1 and run-time 3.8.18.1.
Please respect the Bloomberg licensing agreement and terms of service.
> blpConnect()
> blpAuthenticate()
Warning message:
In blpAuthenticate() : Both ip.address and host are set.  Using ip.address.
> bdp("IBM US Equity", "NAME")
                                     NAME
IBM US Equity INTL BUSINESS MACHINES CORP
>

With SAPI, uuid, auto connect & auth

> testIP <- "x.x.x.x"
> testUUID <- "yyyyyyyy"
> testSAPI <- "z.z.z.z"
>
> options("blpUUID" = testUUID)
> options("blpLoginIP" = testIP)
> options("blpHost" = testSAPI)
> options("blpPort" = 8194)
>
> options("blpAutoConnect" = TRUE)
> options("blpAutoAuthenticate" = TRUE)
> library(Rblpapi, quietly=TRUE)
Rblpapi version 0.3.10 using Blpapi headers 3.8.18.1 and run-time 3.8.18.1.
Please respect the Bloomberg licensing agreement and terms of service.
> bdp("IBM US Equity", "NAME")
                                     NAME
IBM US Equity INTL BUSINESS MACHINES CORP
>

SAPI, uuid, manual connect & auth

> testIP <- "x.x.x.x"
> testUUID <- "yyyyyyyy"
> testSAPI <- "z.z.z.z"
> library(Rblpapi, quietly=TRUE)
Rblpapi version 0.3.10 using Blpapi headers 3.8.18.1 and run-time 3.8.18.1.
Please respect the Bloomberg licensing agreement and terms of service.
> blpConnect(host=testSAPI)
> blpAuthenticate(uuid=testUUID,ip.address=testIP)
> bdp("IBM US Equity", "NAME")
                                     NAME
IBM US Equity INTL BUSINESS MACHINES CORP
>

With DAPI, auto connect

> testDesktop<-"testdesktop"
> options("blpHost" = testDesktop)
> options("blpAutoConnect" = TRUE)
> library(Rblpapi, quietly=TRUE)
Rblpapi version 0.3.10 using Blpapi headers 3.8.18.1 and run-time 3.8.18.1.
Please respect the Bloomberg licensing agreement and terms of service.
> bdp("IBM US Equity", "NAME")
                                     NAME
IBM US Equity INTL BUSINESS MACHINES CORP
>

With DAPI, manual connect

> library(Rblpapi, quietly=TRUE)
Rblpapi version 0.3.10 using Blpapi headers 3.8.18.1 and run-time 3.8.18.1.
Please respect the Bloomberg licensing agreement and terms of service.
> blpConnect(host="testdesktop")
> bdp("IBM US Equity", "NAME")
                                     NAME
IBM US Equity INTL BUSINESS MACHINES CORP
>

SAPI, with Application-name, autoconnect & auth

> options("blpHost" = "x.x.x.x")
> options("blpAppName" = "Foo:Bar")
> options("blpAutoConnect" = TRUE)
> options("blpAutoAuthenticate" = TRUE)
>
> library(Rblpapi, quietly=TRUE)
Rblpapi version 0.3.10 using Blpapi headers 3.8.18.1 and run-time 3.8.18.1.
Please respect the Bloomberg licensing agreement and terms of service.
> bdp("IBM US Equity", "NAME")
                                     NAME
IBM US Equity INTL BUSINESS MACHINES CORP
>

SAPI, Application name, manual connect

> options("blpHost" = "x.x.x.x")
> options("blpAppName" = "Foo:Bar")
> library(Rblpapi, quietly=TRUE)
Rblpapi version 0.3.10 using Blpapi headers 3.8.18.1 and run-time 3.8.18.1.
Please respect the Bloomberg licensing agreement and terms of service.
> blpConnect()
> blpAuthenticate()
> bdp("IBM US Equity", "NAME")
                                     NAME
IBM US Equity INTL BUSINESS MACHINES CORP
>

vignettes/rblpapi-intro.Rmd Show resolved Hide resolved
vignettes/rblpapi-intro.Rmd Outdated Show resolved Hide resolved
ChangeLog Outdated Show resolved Hide resolved
@eddelbuettel
Copy link
Member

eddelbuettel commented Jun 17, 2019

Just quick heads-up to say that this has not fallen off the wagon ... but time is tight, markets are busy and all that. We'll get to it, hopefully 'soon'.

@randomee
Copy link
Contributor Author

No worries. I'll likely be largely MIA in August, just FYI.

@johnlaing
Copy link
Contributor

I refactored some of the interaction between connection/authentication, updated docs, etc. This now seems to be working on my end but it would be good for others to test as well.

@johnlaing
Copy link
Contributor

@randomee how do we look here? It would be good to close this out.

@randomee
Copy link
Contributor Author

Sorry, busy, etc.

The code looks correct, but obviously should be tested. I'll try to test this weekend. I can test them on:

  • DAPI
  • SAPI - no auth - linux
  • SAPI - uuid/ip auth - linux
  • SAPI - appid auth - linux

Hopefully that's enough coverage.

Thanks for bumping this.

@randomee
Copy link
Contributor Author

randomee commented Oct 28, 2019

Ok. Looks good. Tested various ways:

  • auto-connecting
  • manual connection
  • passing id object (for SAPI/uuid and SAPI/appid)
  • using default id object (for SAPI/uuid and SAPI/appid)

Tested these BBG backend types:

  • DAPI - OK
  • SAPI - no auth - linux - OK
  • SAPI - appid auth - linux - OK
  • SAPI - uuid/ip auth - linux - OK

(and this is across several different BBG accounts & infrastructure. so bonus testing.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants