Conversation
woop
requested review from
davidheryanto,
khorshuheng,
pradithya and
zhilingc
as code owners
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: woop The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
woop
force-pushed
the
configuration-refactor
branch
from
03e5673 to
af0918e
Compare
|
Still to do: Need to make all keys in application.yml use a consistent separator |
...ctors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisOnlineRetriever.java
Outdated
Show resolved
Hide resolved
| active_store: online | ||
|
|
||
| # List of store configurations | ||
| stores: |
There was a problem hiding this comment.
As there are several changes in configurations, should this still be ported into v0.4 branch? On the other hand, if we don't port this, any PR that follows after this PR is merged will have a much higher chance of conflicting with the v0.4 branch.
There was a problem hiding this comment.
There are loads of breaking changes. If we backport this then we will have a lot of problems.
any PR that follows after this PR is merged will have a much higher chance of conflicting with the v0.4 branch
That is true, but that is why we maintain a separate branch for 0.4. We should ideally move on from 0.4 because the amount of changes is high and its not worth the upkeep.
There was a problem hiding this comment.
I agree that we should move on from 0.4. But in that case, do we still want to release 0.4.8? And if we do, what should be the last commit that we should stop at? Storage API refactoring, or possibly earlier?
There was a problem hiding this comment.
The point is that we dont backport anything significant. Only critical functionality like hotfixes or patches should be backported. Otherwise there is no point in having releases if we just backport everything.
Refactor, document, and validate Feast Core Properties
Refactor FeastProperties to support nested store configuration
Localize all store configuration in Serving in Spring configuration
Various configuration updates
* Allow Feast Serving to use types properties instead of maps
* Reuse Feast Core Store model in serving
* Remove redundant config classes for Redis
* Update Serving Beans and Config classes to use ne1w configuration getters
* Remove hot-loading from store configuration. This reduces a bit of
flexibility, but simplifies the code and configuration
…rvingServiceConfig
woop
force-pushed
the
configuration-refactor
branch
from
f3b47b3 to
e2b43cc
Compare
|
/lgtm |
4 participants
What this PR does / why we need it:
This PR is meant to resolve #525 and should be merged on only after the #567 and #533 PRs have been merged in.
The major changes will be that it will restructure and clean up Feast configuration properties in order for developers to find it easier to configure a Feast deployment and easier to know when it is misconfigured.
Additionally I am attempting to create a consistent way to instantiate and configure stores.
Which issue(s) this PR fixes:
Fixes #525
Does this PR introduce a user-facing change?: