Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

remove unnecessary spark job from registry test#790

Merged
blrchen merged 1 commit into
feathr-ai:mainfeathr-ai/feathr:mainfrom
Yuqing-cat:citestfixYuqing-cat/feathr:citestfixCopy head branch name to clipboard
Oct 31, 2022
Merged

remove unnecessary spark job from registry test#790
blrchen merged 1 commit into
feathr-ai:mainfeathr-ai/feathr:mainfrom
Yuqing-cat:citestfixYuqing-cat/feathr:citestfixCopy head branch name to clipboard

Conversation

@Yuqing-cat
Copy link
Copy Markdown
Collaborator

Signed-off-by: Yuqing Wei weiyuqing021@outlook.com

Description

Based on CI failure here: https://github.com/feathr-ai/feathr/actions/runs/3279090567/jobs/5398225684#step:8:510

feathr_project/test/test_feature_registry.py is marked as F but the RuntimeError: Spark job failed. has nothing todo with registry itself.

By removing the unnecessary spark job, test cost will be saved and only feature register use case will be tested.

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.

Signed-off-by: Yuqing Wei <weiyuqing021@outlook.com>
@blrchen blrchen requested a review from enya-yx October 28, 2022 06:47
@xiaoyongzhu
Copy link
Copy Markdown
Member

@Yuqing-cat assuming we have other test cases to cover the E2E test for registry?

@blrchen
Copy link
Copy Markdown
Collaborator

blrchen commented Oct 28, 2022

@Yuqing-cat assuming we have other test cases to cover the E2E test for registry?

The daily notebook test covers registry E2E, given CI now is taking very long to complete and spark job fails frequently, we should make CI test more scoped and united.

@blrchen blrchen merged commit 2f868ef into feathr-ai:main Oct 31, 2022
blrchen added a commit that referenced this pull request Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.