-
Notifications
You must be signed in to change notification settings - Fork 3
feat: Remote Registry Support #294
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
base: master
Are you sure you want to change the base?
Conversation
EXPEbdodla
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a test cases. Rest all looking good to me.
0082406 to
778bcb7
Compare
| applyCommand.Dir = featureRepoPath | ||
| out, err := applyCommand.CombinedOutput() | ||
| if err != nil { | ||
| log.Printf("Repo setup error: %s", string(out)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log error as well.
| // Verify registry.db was created with retries | ||
| registryPath := filepath.Join(dataDir, "registry.db") | ||
| for i := 0; i < 10; i++ { | ||
| if _, err := os.Stat(registryPath); err == nil { | ||
| log.Printf("Registry file created successfully after %d attempts", i+1) | ||
| break | ||
| } | ||
| if i == 9 { | ||
| return fmt.Errorf("registry.db was not created after feast apply") | ||
| } | ||
| time.Sleep(1 * time.Second) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move it to else.
|
|
||
| message ExpediaSearchFeatureViewsRequest { | ||
| string search_text = 1; | ||
| google.protobuf.BoolValue online = 2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
write a comment when we are not using bool type.
| int32 total_projects = 3; | ||
| int32 total_page_indices = 4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reset indexes.
| def ExpediaSearchFeatureViews( | ||
| self, request: RegistryServer_pb2.ExpediaSearchFeatureViewsRequest, context | ||
| ): | ||
| # Using `type: ignore[attr-defined]` because this should only be implemented in sql registry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
proxied_registry should be SQL Registry or SQL Fallback registry. Throw error if its different registry type.
| path="/tmp/multi_view_sink", | ||
| file_format="parquet", | ||
| timestamp_field="daily_driver_stats__event_timestamp", | ||
| created_timestamp_column="daily_driver_stats__created", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to remove created_timestamp_column?
|
|
||
| @pytest.fixture(scope="session") | ||
| def sqlite_registry(): | ||
| @pytest.fixture(scope="function") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check why we need this.
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes https://expediagroup.atlassian.net/browse/EAPC-19174
Misc