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

Feature/readsecrets + brief readme #6

Open
wants to merge 41 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
e2bd89c
quick fix
Apr 3, 2020
c71e92f
added comments
Apr 3, 2020
033358e
added absolute path
Apr 3, 2020
c9f9732
added some usage docs
Apr 3, 2020
e6c7ac4
cascading secrets
Apr 3, 2020
8e895b8
optional client constructor parameters
Apr 3, 2020
3220f70
better error reporting
Apr 3, 2020
923bc00
removed superflous commentary
Apr 3, 2020
c37c1d8
added portfolio delete test + holdings test inception
Apr 3, 2020
158f1c8
ES6 upgrades + nicer logging
Apr 3, 2020
ba45d72
holdings interim
Apr 3, 2020
dc50d15
holdings interim
Apr 5, 2020
0e890ba
holdings assertions alpha
Apr 5, 2020
970cc0f
removal of unused reference
Apr 5, 2020
6144dd0
lusidTools inception + defineBuyRequest
Apr 6, 2020
7ff33c9
getHoldings beta
Apr 6, 2020
6656321
adjust holdings beta
Apr 6, 2020
bc466bd
delete back in test
Apr 6, 2020
84c4f30
portfolios beta
Apr 6, 2020
11b624a
better title
Apr 6, 2020
ade1a80
made error reporting broader
Apr 6, 2020
d25862c
broader error reporting
Apr 6, 2020
7ccd1f3
removed unused libraries
Apr 6, 2020
90ab2fc
defaults update
Apr 6, 2020
1986cd1
fallback beta
Apr 7, 2020
581abff
configuration
Apr 7, 2020
de7e7b7
removed comments
Apr 7, 2020
5a144d8
+comments
Apr 7, 2020
63a7fb0
explicit fallbacks
Apr 7, 2020
2e7a030
oauth2
Apr 7, 2020
d31c5ab
lusidUtilities
Apr 7, 2020
b310b89
elegant update
Apr 7, 2020
38b4024
fix docs
Apr 7, 2020
c44409f
one transaction test
Apr 7, 2020
abefcf1
improvements
Apr 8, 2020
d872778
createTransactions beta
Apr 8, 2020
bbb0e73
bitemporal alpha
Apr 8, 2020
a226e38
bitemporal with JS patch
Apr 8, 2020
3ef9303
cut labels alpha
Apr 8, 2020
c71e870
cut labels alpha
Apr 9, 2020
7da1325
cut labels alpha
Apr 9, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 20 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,23 @@ If you would prefer to generate the Javsscript SDK locally from the FINBOURNE Op
- download the latest swagger.json file from https://api.lusid.com/swagger/index.html
- save it in this directory as lusid.json
- run `$ docker-compose up && docker-compose rm -f`

# Working with the SDK

The relevant tests are in sdk/test/tutorials

You need to either create a sdk/secrets.json file, or set environment variables, based on your lusid installation. The configurations go into sdk/test/tutorials/clientBuilder.ts

To generate the JS based on TS

```
$ cd sdk/
$ npm i
$ npm run-script build
```

To run all scripts in sdk/test/tutorials

```
$ npm test
```
4 changes: 2 additions & 2 deletions sdk/client/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {Api} from "../apis";


// Set the default path to the secrets file
const secretsPath = './secrets.json'
const secretsPath = process.cwd() + '/secrets.json'
victorfinbourne marked this conversation as resolved.
Show resolved Hide resolved
// Set the default amount of seconds before token expiry to call a refresh
const refreshLimit = 3580

Expand Down Expand Up @@ -258,7 +258,7 @@ export class Client {
} else if (source = Source.Secrets) {
// Try and import the credentials file
try {
var credentials: any = require(this.secretsFilePath)
var credentials: any = require(this.secretsFilePath).api
victorfinbourne marked this conversation as resolved.
Show resolved Hide resolved
// If there are any problems throw an error
} catch (e) {
throw "File " + this.secretsFilePath + " could not be found/read, ensure that it exists in the client folder"
Expand Down
31 changes: 24 additions & 7 deletions sdk/test/tutorials/clientBuilder.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,27 @@
import { Client, Source } from '../../client/client'

/*
Read from Environment Variables
*/

// export var client = new Client(
// [Source.Environment, 'FBN_TOKEN_URL'],
// [Source.Environment, 'FBN_USERNAME'],
// [Source.Environment, 'FBN_PASSWORD'],
// [Source.Environment, 'FBN_CLIENT_ID'],
// [Source.Environment, 'FBN_CLIENT_SECRET'],
// [Source.Environment, 'FBN_LUSID_API_URL'],
// )

/*
Read from secrets.json
*/

export var client = new Client(
[Source.Environment, 'FBN_TOKEN_URL'],
[Source.Environment, 'FBN_USERNAME'],
[Source.Environment, 'FBN_PASSWORD'],
[Source.Environment, 'FBN_CLIENT_ID'],
[Source.Environment, 'FBN_CLIENT_SECRET'],
[Source.Environment, 'FBN_LUSID_API_URL'],
)
[Source.Secrets, 'tokenUrl'],
[Source.Secrets, 'username'],
[Source.Secrets, 'password'],
[Source.Secrets, 'clientId'],
[Source.Secrets, 'clientSecret'],
[Source.Secrets, 'apiUrl']
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this client runs the tests we need some way to ensure that we can run them locally using a secrets file and in our continuous integration pipeline which will use environment variables.

The logic in the Python SDK is to use a secrets file if provided, falling back to environment variables if it can't find a secret file. Do you think we could implement similar logic here?

https://github.com/finbourne/lusid-sdk-python-preview/tree/master/sdk/lusid/utilities

I would suggest that this lives in the sdk/client/client.ts but then you can delete completely the commented out code which references environment variables.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep. will do.