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.

Sign up for GitHub

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

Jump to bottom

docs: ADR Quill library for sql statement generation #318

Merged
merged 4 commits into from
Jan 30, 2023
Merged

docs: ADR Quill library for sql statement generation #318

merged 4 commits into from
Jan 30, 2023

Conversation

yshyn-iohk
Copy link
Member

@yshyn-iohk yshyn-iohk commented Jan 19, 2023 •
edited

Overview

ADR for using Quill library

Checklist

My PR contains...

  • No code changes (changes to documentation, CI, metadata, etc.)
  • Bug fixes (non-breaking change which fixes an issue)
  • Improvements (misc. changes to existing features)
  • Features (non-breaking change which adds functionality)

My changes...

  • are breaking changes
  • are not breaking changes
  • If yes to above: I have updated the documentation accordingly

Documentation

  • My changes do not require a change to the project documentation
  • My changes require a change to the project documentation
  • If yes to above: I have updated the documentation accordingly

Tests

  • My changes can not or do not need to be tested
  • My changes can and should be tested by unit and/or integration tests
  • If yes to above: I have added tests to cover my changes
  • If yes to above: I have taken care to cover edge cases in my tests

@yshyn-iohk yshyn-iohk requested review from a team, atala-dev and essbante-io and removed request for a team January 19, 2023 14:33
@atala-dev
Copy link
Contributor

atala-dev commented Jan 19, 2023 •
edited

🦙 MegaLinter status: ⚠️ WARNING

Descriptor Linter Files Fixed Errors
⚠️ MARKDOWN markdownlint 2 21
⚠️ MARKDOWN markdown-link-check 2 4
✅ MARKDOWN markdown-table-formatter 2 0
✅ REPOSITORY dustilock yes no
✅ REPOSITORY git_diff yes no
✅ REPOSITORY syft yes no
⚠️ YAML prettier 1 1
✅ YAML v8r 1 0
✅ YAML yamllint 1 0

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by OX Security

bvoiturier
bvoiturier reviewed Jan 19, 2023
View reviewed changes
Copy link
Contributor

@bvoiturier bvoiturier left a comment

Choose a reason for hiding this comment

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

Given Quill (without Doobie) is the only option bringing async/non-blocking IO support, I would be tempted to go for that solution right now.

I don't think it would make such a big difference in terms of refactoring time. We have to refactor anyway, and non-blocking IO is really something we want to have all the way down from the REST interface to the database.

What do you think?

@yshyn-iohk
Copy link
Member Author

yshyn-iohk commented Jan 20, 2023 •
edited

@bvoiturier, it's hard to say as this approach really required additional efforts for testing and some PoC.
From the getquill site:

If you like to “live dangerously” and want to try a socket-level async library, try quill-jasync-postgres or quill-jasync-mysql.

That's why I currently propose to stay with doobie for query execution.
But, it would be the best solution from the architectural point of view, so let's discuss this with the team.

@yshyn-iohk yshyn-iohk closed this Jan 20, 2023
@yshyn-iohk yshyn-iohk reopened this Jan 20, 2023
@bvoiturier
Copy link
Contributor

@yshyn-iohk, OK I see your point. There seem to be 2 options to handle non-blocking IO in Quill: quill-async and quill-jasync modules. Would be worth planning a spike at some point to give it a try. Will add this as a topic for the next AOH.

@yshyn-iohk
Copy link
Member Author

@bvoiturier, I would wait for the first performance test results. If doobie with the connection pooling is not enough, we definitely will switch to quill-jasync library and switch to option 3 of this ADR
doobie is a battle-tested library, when using quill-jasync might be risky
as I quoted in my previous comment:

If you like to “live dangerously” and want to try a socket-level async library, try quill-jasync-postgres

FabioPinheiro
FabioPinheiro approved these changes Jan 27, 2023
View reviewed changes
Copy link
Contributor

@FabioPinheiro FabioPinheiro left a comment

Choose a reason for hiding this comment

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

LGTM

.mega-linter.yml
@@ -14,7 +14,7 @@ DISABLE_LINTERS:
SCALA_SCALAFIX,
SQL_TSQLLINT,
]
DISABLE_ERRORS_LINTERS: [KOTLIN_KTLINT, PROTOBUF_PROTOLINT, OPENAPI_SPECTRAL]
DISABLE_ERRORS_LINTERS: [KOTLIN_KTLINT, PROTOBUF_PROTOLINT, OPENAPI_SPECTRAL, MARKDOWN_MARKDOWN_LINK_CHECK]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we disabling the MARKDOWN_MARKDOWN_LINK_CHECK?

Copy link
Member Author

Choose a reason for hiding this comment

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

The links to closed repositories in GitHub return a 404 error, and the linter fails the build.

@yshyn-iohk yshyn-iohk merged commit edaab33 into main Jan 30, 2023
@yshyn-iohk yshyn-iohk deleted the adr-quill-library branch January 30, 2023 08:38
yshyn-iohk added a commit that referenced this pull request Aug 18, 2023
@yshyn-iohk
I, Yurii Shynbuiev hereby sign-off-by all of my past commits to this …
4538766 
…repo subject to the Developer Certificate of Origin (DCO), Version 1.1.

81a2ad9 docs: removed secret-storage from the Tutorials section. (#559) [skip-ci]
130abbd docs: fix secret storage documentation page (#556)
4f413a3 docs: add secret management documentation [skip-ci] (#542)
885870d Fix ATL-4830 verification policies update (#549)
f6d1fd4 docs: compose the ADR for DID-linked resources. ATL-3186 [skip-ci] (#518)
dc54af8 docs: ADR for data isolation for multi-tenancy (#531)
357273d docs: ADR for HD key derivation in the PRISM v2 (#529)
64b0a2a fix(prism-agent): Alight the error responses according to RFC7807. ATL-3962 (#480)
7fa2e1a fix(prism-agent) Align VerificationPolicy OAS ATL-3909 (#473)
8d42fb1 fix: update mercury to 0.21.0
979b609 chore: update mercury dependency to 0.21.0 (#458)
d3a8d15 feat(prism-agent) update schema logic - agent part. ATL-3164  (#452)
6e22bfc feat(pollux): update credential schema logic (#450) ATL-3164
68b8e4a docs: credential schema introduction, creation, update, and delete. ATL-3548 (#443)
91902ce feat(prism-agent): Add OAS specification to the schema registry endpoint. ATL-3164 (#438)
32f9e83 feat(prism-agent): CredentialSchema DAL, model, service and repositor… (#425)
79352f0 feat(pollux): CredentialSchema DAL, model, service and repository #2 (#424)
6e941da docs(pollux): add credential-schema.md to docusaurus (#407)
ffa5f7e feat(pollux): CredentialSchema service, repository and sql (#416)
c96f804 doc(pollux): verification policy documentation (#384)
1e47ba1 doc(pollux): add schema registry documentation. ATL-1334 (#296)
142ff55 feat(prism-agent): integrate VerificationPolicy DAL into the agent, update OAS and implement REST API for verification policies (#369)
b290a18 feat(pollux): implement the DAL for CRUD on the verifiable policy entity. ATL-2478 (#368)
edaab33 docs: ADR Quill library for sql statement generation (#318)
3d0c642 feat(prism-agent): implement DAL for the credential schema. ATL-1334
e0831e8 feat(pollux): fix the lookup count in the credential schema DAL (#315)
f43320f feat(pollux): add dal for the credential schema ATL-1342 (#298)
6903afa fix(prism-agent): switch datetime format to offsetdatetime. ATL-2723 (#243)
e13a1bd fix(connect): bump mercury version to 1.10.1 and touch README.md
ee27755 fix(pollux): upgrade mercury lib to 1.10.1
cdd4772 fix(castor): README.md is added to increase the version of the castor library
5ffb0cc fix(mercury): simple commit to increase the version of mercury library
403eb38 feat(prism-agent): verification policies pagination. ATL-1334 (#205)
726e2d9 feat(prism-agent): implement pagination with navigation for schema-registry (#195)
4528c57 feat(pollux): alight the OAS for schema registry (#189)
16d5fdb feat(pollux): cleanup the code of IssueCredentialApi
79170f8 feat(pollux): cleanup the OAS from Issue Credentials and other unused tags
d75b36b feat(pollux): schema registry lookup and verification policies REST API ATL-1334 (#168)
b3cf828 feat(apollo): add schema registry to the agent using Tapir library. ATL-1334  (#94)
8d8bf56 doc(adr): use Tapir library as a DLS for OAS (#51)
496337b ATL-1334 feat(pollux): add sandbox project to play with Tapir and schema-registry (#62)
95667ba doc(pollux): add JWT encoding and Present Proof endpoints (#37)
234bc06 [ATL-1388] doc(pollux): add Revocation Registry API, remove v1 from the path, apply changes after review (#14)
e47242c [ATL-1001] doc: add examples of OpenAPI specification of well-known competitors (#23)
22e37cf doc(pollux): add Pollux open-api specification (schema and issue-credentials) (#8)

Signed-off-by: Yurii Shynbuiev <yurii.shynbuiev@iohk.io>
@yshyn-iohk yshyn-iohk mentioned this pull request Aug 18, 2023
Merged
antonbaliasnikov pushed a commit that referenced this pull request Aug 21, 2023
@yshyn-iohk
I, Yurii Shynbuiev hereby sign-off-by all of my past commits to this …
cb8cd06 
…repo subject to the Developer Certificate of Origin (DCO), Version 1.1.

81a2ad9 docs: removed secret-storage from the Tutorials section. (#559) [skip-ci]
130abbd docs: fix secret storage documentation page (#556)
4f413a3 docs: add secret management documentation [skip-ci] (#542)
885870d Fix ATL-4830 verification policies update (#549)
f6d1fd4 docs: compose the ADR for DID-linked resources. ATL-3186 [skip-ci] (#518)
dc54af8 docs: ADR for data isolation for multi-tenancy (#531)
357273d docs: ADR for HD key derivation in the PRISM v2 (#529)
64b0a2a fix(prism-agent): Alight the error responses according to RFC7807. ATL-3962 (#480)
7fa2e1a fix(prism-agent) Align VerificationPolicy OAS ATL-3909 (#473)
8d42fb1 fix: update mercury to 0.21.0
979b609 chore: update mercury dependency to 0.21.0 (#458)
d3a8d15 feat(prism-agent) update schema logic - agent part. ATL-3164  (#452)
6e22bfc feat(pollux): update credential schema logic (#450) ATL-3164
68b8e4a docs: credential schema introduction, creation, update, and delete. ATL-3548 (#443)
91902ce feat(prism-agent): Add OAS specification to the schema registry endpoint. ATL-3164 (#438)
32f9e83 feat(prism-agent): CredentialSchema DAL, model, service and repositor… (#425)
79352f0 feat(pollux): CredentialSchema DAL, model, service and repository #2 (#424)
6e941da docs(pollux): add credential-schema.md to docusaurus (#407)
ffa5f7e feat(pollux): CredentialSchema service, repository and sql (#416)
c96f804 doc(pollux): verification policy documentation (#384)
1e47ba1 doc(pollux): add schema registry documentation. ATL-1334 (#296)
142ff55 feat(prism-agent): integrate VerificationPolicy DAL into the agent, update OAS and implement REST API for verification policies (#369)
b290a18 feat(pollux): implement the DAL for CRUD on the verifiable policy entity. ATL-2478 (#368)
edaab33 docs: ADR Quill library for sql statement generation (#318)
3d0c642 feat(prism-agent): implement DAL for the credential schema. ATL-1334
e0831e8 feat(pollux): fix the lookup count in the credential schema DAL (#315)
f43320f feat(pollux): add dal for the credential schema ATL-1342 (#298)
6903afa fix(prism-agent): switch datetime format to offsetdatetime. ATL-2723 (#243)
e13a1bd fix(connect): bump mercury version to 1.10.1 and touch README.md
ee27755 fix(pollux): upgrade mercury lib to 1.10.1
cdd4772 fix(castor): README.md is added to increase the version of the castor library
5ffb0cc fix(mercury): simple commit to increase the version of mercury library
403eb38 feat(prism-agent): verification policies pagination. ATL-1334 (#205)
726e2d9 feat(prism-agent): implement pagination with navigation for schema-registry (#195)
4528c57 feat(pollux): alight the OAS for schema registry (#189)
16d5fdb feat(pollux): cleanup the code of IssueCredentialApi
79170f8 feat(pollux): cleanup the OAS from Issue Credentials and other unused tags
d75b36b feat(pollux): schema registry lookup and verification policies REST API ATL-1334 (#168)
b3cf828 feat(apollo): add schema registry to the agent using Tapir library. ATL-1334  (#94)
8d8bf56 doc(adr): use Tapir library as a DLS for OAS (#51)
496337b ATL-1334 feat(pollux): add sandbox project to play with Tapir and schema-registry (#62)
95667ba doc(pollux): add JWT encoding and Present Proof endpoints (#37)
234bc06 [ATL-1388] doc(pollux): add Revocation Registry API, remove v1 from the path, apply changes after review (#14)
e47242c [ATL-1001] doc: add examples of OpenAPI specification of well-known competitors (#23)
22e37cf doc(pollux): add Pollux open-api specification (schema and issue-credentials) (#8)

Signed-off-by: Yurii Shynbuiev <yurii.shynbuiev@iohk.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bvoiturier bvoiturier bvoiturier left review comments

@patlo-iog patlo-iog patlo-iog left review comments

@FabioPinheiro FabioPinheiro FabioPinheiro approved these changes

@atala-dev atala-dev Awaiting requested review from atala-dev

@essbante-io essbante-io Awaiting requested review from essbante-io

Assignees
No one assigned
Labels
Type: Documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@yshyn-iohk @atala-dev @bvoiturier @FabioPinheiro @patlo-iog