Skip to content

Conversation

@xiaoyongzhu
Copy link
Member

@xiaoyongzhu xiaoyongzhu commented Nov 7, 2022

Description

This PR decouples the save_to_feature_config_from_context method from the registry code, given:

  1. It doesn't make sense to have this method in the registry code
  2. This method introduces unnecessary dependencies for registry, which makes a few other functionalities have dependencies on feature registry, such as local spark runner/multi cloud support.

How was this PR tested?

Does this PR introduce any user-facing changes?

  • No. You can skip the rest of this section.
  • Yes. Make sure to clarify your proposed changes.

@xiaoyongzhu xiaoyongzhu added the safe to test Tag to execute build pipeline for a PR from forked repo label Nov 7, 2022
Copy link
Collaborator

@Yuqing-cat Yuqing-cat left a comment

Choose a reason for hiding this comment

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

Looks good to me, add some comments.

@xiaoyongzhu xiaoyongzhu merged commit 799fac0 into feathr-ai:main Nov 23, 2022
@xiaoyongzhu xiaoyongzhu deleted the decouple_build_feature branch November 23, 2022 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Tag to execute build pipeline for a PR from forked repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants