-
Notifications
You must be signed in to change notification settings - Fork 283
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
feat(azure-kv): added keychain plugin for azure keyvault #991
feat(azure-kv): added keychain plugin for azure keyvault #991
Conversation
273c69e
to
35e7942
Compare
...s-plugin-keychain-azure-kv/src/main/typescript/generated/openapi/typescript-axios/.gitignore
Outdated
Show resolved
Hide resolved
...s-plugin-keychain-azure-kv/src/main/typescript/generated/openapi/typescript-axios/.npmignore
Outdated
Show resolved
Hide resolved
packages/cactus-plugin-keychain-azure-kv/src/main/typescript/plugin-keychain-azure-kv.ts
Outdated
Show resolved
Hide resolved
packages/cactus-plugin-keychain-azure-kv/src/main/typescript/plugin-keychain-azure-kv.ts
Outdated
Show resolved
Hide resolved
packages/cactus-plugin-keychain-azure-kv/src/main/typescript/plugin-keychain-azure-kv.ts
Outdated
Show resolved
Hide resolved
...us-plugin-keychain-azure-kv/src/test/typescript/integration/plugin-keychain-azure-kv.test.ts
Show resolved
Hide resolved
packages/cactus-plugin-keychain-azure-kv/src/test/typescript/unit/api-surface.ts
Outdated
Show resolved
Hide resolved
b3748f1
to
aec199e
Compare
@jagpreetsinghsasan See also this for a good example on mocking for unit tests: https://github.com/petermetz/cactus/tree/feat-object-store-plugin-ipfs packages/cactus-plugin-object-store-ipfs/src/test/typescript/fixtures/mock/ipfs/ipfs-files-api-mock.ts |
3a06dc8
to
b58059a
Compare
packages/cactus-plugin-keychain-azure-kv/src/main/json/openapi.json
Outdated
Show resolved
Hide resolved
packages/cactus-plugin-keychain-azure-kv/src/main/json/openapi.json
Outdated
Show resolved
Hide resolved
packages/cactus-plugin-keychain-azure-kv/src/main/json/openapi.json
Outdated
Show resolved
Hide resolved
packages/cactus-plugin-keychain-azure-kv/src/main/typescript/plugin-keychain-azure-kv.ts
Outdated
Show resolved
Hide resolved
packages/cactus-plugin-keychain-azure-kv/src/main/typescript/plugin-keychain-azure-kv.ts
Outdated
Show resolved
Hide resolved
...us-plugin-keychain-azure-kv/src/test/typescript/integration/plugin-keychain-azure-kv.test.ts
Outdated
Show resolved
Hide resolved
...us-plugin-keychain-azure-kv/src/test/typescript/integration/plugin-keychain-azure-kv.test.ts
Outdated
Show resolved
Hide resolved
...es/cactus-plugin-keychain-azure-kv/src/test/typescript/mock/plugin-keychain-azure-kv-mock.ts
Outdated
Show resolved
Hide resolved
b58059a
to
f5a7455
Compare
@jagpreetsinghsasan Should I give this one another go? (I see the code changes but you haven't yet clicked re-request review) |
packages/cactus-plugin-keychain-azure-kv/src/main/json/openapi.json
Outdated
Show resolved
Hide resolved
3d4f43c
to
431c5af
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@elenaizaguirre @jagpreetsinghsasan Please coordinate your PRs merging in order to make sure that the openapi.json spec references are updated "at the right time" depending on who's PR gets merged first.
Context: Elena is working on a change that necessitates for us to use the http URLs instead of relative file paths.
Codecov Report
@@ Coverage Diff @@
## main #991 +/- ##
==========================================
- Coverage 73.30% 71.71% -1.60%
==========================================
Files 248 270 +22
Lines 8782 9411 +629
Branches 1025 1114 +89
==========================================
+ Hits 6438 6749 +311
- Misses 1796 2087 +291
- Partials 548 575 +27
Continue to review full report at Codecov.
|
Would you wait my review since I'm asking about this PR on #1017? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jagpreetsinghsasan Please update to all Cactus package version occurrences to 0.6.0 from 0.5.0 (just issued the release) and then run the configure script to also update the lock files as well. Commit all of those changes and then we are ready to merge. Don't forget the squash the commits into a single one.
66162ed
to
3a9304a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jagpreetsinghsasan Almost forgot, please also
- migrate the tsconfig.json inside your package to match the new boilerplate (see any of the other packages for an example).
- Add an entry to the root tsconfig.json file under the
references
array that points to the new package that is being added in the PR - Do this to all the pending PRs that are adding new packages
c142385
to
b48d11a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jagpreetsinghsasan LGTM, please rebase onto latest upstream/main to resolve the conflict(s) and then we are good to go.
b48d11a
to
73b62bb
Compare
I just resolved the conflict because it was needed for Jeffrey's dependent PR as well. |
Primary Change --- 1. Added new package cactus-plugin-keychain-azure-kv under packages/ 2. Added PluginKeychainAzureKvMock class to mock the functions of SecretClient under packages/cactus-plugin-keychain-azure-kv/src/test/typescript/mock/plugin-keychain-azure-kv-mock.ts Resolves hyperledger-cacti#971 Signed-off-by: jagpreetsinghsaan <jagpreet.singh.sasan@accenture.com> Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
Commit to be reviewed
feat(azure-kv): added keychain plugin for azure keyvault
Resolves #971
Signed-off-by: Jagpreet Singh Sasan jagpreet.singh.sasan@accenture.com