search

Rblp / Rblpapi

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

This change expands B-PIPE coverage in Rblpapi to cover the use case #369

Closed rharlow86 closed 1 year ago

rharlow86 commented 1 year ago

of a user authenticating with a B-PIPE application with an EMRS ID and an IP address. Rather than creating an entirely new authenticate method, add an isEmrsId parameter to determine whether a uuid is in fact an emrsId rather than a uuid.

closes #342

eddelbuettel commented 1 year ago

Nice work. I added a few minor edits nits you could quickly add (and then of course also run compileAttributes() and roxygenize() to carry a signature change forward) but I don't even think these are "must fix" type changes.

It would be nice to hear from one more (@johnlaing , @armstrtw -- if you have a moment) before merging.

eddelbuettel commented 1 year ago

Oh, one thing though: Would you mind adding a ChangeLog entry with a line per changed file -- you'll see example in the file itself. And there are two spaces between the three fields date + name + email. Thanks!

rharlow86 commented 1 year ago

@eddelbuettel thanks for taking a look! Should be able to make the updates in the next few days. Also, since I do have access to BPIPE I'd be happy to volunteer for testing for future patches from others.

rharlow86 commented 1 year ago

I took a further look at this today, and while data appears to be flowing through just fine with this patch, I would like to do a little bit more testing on my end to ensure that this is really doing what I think it's doing in terms of authorization/authentication.

eddelbuettel commented 1 year ago

Take your time -- there is no rush. Aiming for solid code quality and behavior is good and worth a wait.

rharlow86 commented 1 year ago

@eddelbuettel After a bit more testing on my end, I realized that I need to add one more parameter associated with the "appName" both at the R level & the C++ level. But overall, this is very similar to what I had done before, just with one new required parameter. I have now confirmed that this is definitely behaving as it should (the user must be logged into a terminal at the stated IP address and also registered to be a "member" of the stated application name).