RETIRED: Hibernate Migration (replaced by 543) #532
Conversation
RobGeada
requested review from
ruivieira,
tteofili and
christinaexyou
and removed request for
a team
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
PR image build and manifest generation completed successfully! 📦 PR image: 🗂️ CI manifests |
|
/test trustyai-explainability-e2e |
| <dependency> | ||
| <groupId>io.quarkus</groupId> | ||
| <artifactId>quarkus-hibernate-orm</artifactId> | ||
| <version>3.2.11.Final</version> | ||
| </dependency> |
There was a problem hiding this comment.
I would try to keep explainability-core thin so I prefer to have a explainability-persistance module to compose this use case if possible.
| <dependency> | ||
| <groupId>io.quarkus</groupId> | ||
| <artifactId>quarkus-hibernate-orm</artifactId> | ||
| <version>3.2.11.Final</version> |
There was a problem hiding this comment.
This should be managed via the import of quarkus bom instead of explicit version (this version is tied to quarkus version)
| /** | ||
| * Wrapper class for any kind of value part of a prediction input or output. | ||
| * | ||
| */ | ||
| @Embeddable |
There was a problem hiding this comment.
If you only need similar annotation in core you can add jakarta-persistance-api dependency
...lity-core/src/main/java/org/kie/trustyai/explainability/model/dataframe/DataframeColumn.java
Show resolved
Hide resolved
...ore/src/main/java/org/kie/trustyai/explainability/model/dataframe/DataframeInternalData.java
Show resolved
Hide resolved
| @CacheResult(cacheName = "dataframe", keyGenerator = DataCacheKeyGen.class) | ||
| Dataframe readData(String modelId) throws StorageReadException; |
There was a problem hiding this comment.
This is mixing storage and caching, we need to pay attention to this while we do performance test otherwise we have wrong/fake result
There was a problem hiding this comment.
Yep, I'm aware of this, that's why performance tests always retrieve novel dfs, never the same df twice
...ainability-core/src/main/java/org/kie/trustyai/explainability/model/dataframe/Dataframe.java
Show resolved
Hide resolved
explainability-service/src/main/java/org/kie/trustyai/service/data/storage/Storage.java
Show resolved
Hide resolved
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
RobGeada
changed the title
Many thanks for submitting your Pull Request ❤️!
Please make sure that your PR meets the following requirements:
FAI-XYZ Subject[0.3.x] FAI-XYZ Subject