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

ADR-0001: TrustyAI external library integration #2

Merged
merged 12 commits into from
May 15, 2023

Conversation

ruivieira
Copy link
Member

@ruivieira ruivieira commented May 2, 2023

Architecture Decision Record for external library integration.

The goal of this PR is to discuss which proposal to take forward.

Copy link
Member

@tteofili tteofili left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@danielezonca danielezonca left a comment

Choose a reason for hiding this comment

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

Thanks @ruivieira for the proposal
It looks good to me, just some minor comments to clarify some aspects.

My votes (see Apache Voting 😄 ):

Proposal 1: +0
Probably a bit easier to implement (also because it is not going to add new deployments) but harder to extend/compose

Proposal 2 +1
It might be a bit more complex but overall I find it more clean and "future proof"

Final note:
If necessary we implement also proposal 1 in the future with some "dynamic loading" mechanism (via ServiceLoader) inside TrustyAI Service.

ruivieira and others added 5 commits May 5, 2023 13:15
Co-authored-by: Daniele Zonca <dzonca@redhat.com>
Co-authored-by: Daniele Zonca <dzonca@redhat.com>
Co-authored-by: Daniele Zonca <dzonca@redhat.com>
Co-authored-by: Daniele Zonca <dzonca@redhat.com>
@ruivieira ruivieira added the ADR/under-discussion Architecture Decision Record under discussion label May 5, 2023
adr/README.md Outdated Show resolved Hide resolved
@ruivieira
Copy link
Member Author

5855ef1 should read "Move ADR-0001 proposal 2 into the Considered section"

@ruivieira ruivieira merged commit 2a413fb into trustyai-explainability:main May 15, 2023
@ruivieira ruivieira removed the ADR/under-discussion Architecture Decision Record under discussion label May 15, 2023
@ruivieira ruivieira deleted the adr-0001 branch May 15, 2023 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ADR/approved ADR Architecture Decision Record
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants