search

openwallet-foundation / askar

Secure storage designed for Hyperledger Aries agents.
Apache License 2.0
63 stars 51 forks source link

'descending #317

Open scottexton opened 1 month ago

scottexton commented 1 month ago

A recent commit changed the Store.scan and Session.fetchAll JavaScript APIs so that the new 'descending' field is now required. Was this planned, or by mistake? Shouldn't we just have a default like there is for the other options.

swcurran commented 1 month ago

@ff137 — I’m guessing that was a change you made? ^^^^

ff137 commented 1 month ago

@scottexton the relevant changes were made here: https://github.com/hyperledger/aries-askar/pull/291/files

My intention was for the new fields to be optional / have defaults, so that there are no breaking changes. I think I managed that in the rust/python code, but not in the JavaScript wrapper... apologies.

Looks like descending: boolean should be descending?: boolean in the wrappers/javascript/packages/aries-askar-shared methods.

scottexton commented 1 month ago

@ff137 Will you make a fix for that, or do you want me to?

ff137 commented 1 month ago

@scottexton I do feel responsible to clean up my mess and make a PR, but review-approve takes time. It's probably best if you can test a fix, given that you're working with the js wrapper.

Should just involve adding a ? to those descending: bool lines

scottexton commented 1 month ago

@ff137 Alright. I will include a fix in amongst my other changes.

scottexton commented 4 weeks ago

A pull request has been created with a fix for this issue (https://github.com/hyperledger/aries-askar/pull/319) - this fix is embedded within a larger change to introduce ODBC backend support.