Skip to content
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

Primary Table Registration Refactor #1428

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

epps
Copy link
Contributor

@epps epps commented Apr 18, 2024

Description

This PR enhances the types, metadata, protos to better express primary data source in cloud object stores.

Type of change

Select type(s) of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have fixed any merge conflicts

@epps epps requested a review from simba-git April 18, 2024 18:54
@epps epps temporarily deployed to Integration testing April 18, 2024 18:54 — with GitHub Actions Inactive
Copy link

codecov bot commented Apr 18, 2024

Codecov Report

Attention: Patch coverage is 51.97889% with 182 lines in your changes are missing coverage. Please review.

Project coverage is 57.25%. Comparing base (dee2c5c) to head (3b6531a).
Report is 1 commits behind head on main.

Files Patch % Lines
provider/spark.go 7.31% 37 Missing and 1 partial ⚠️
metadata/client.go 67.27% 16 Missing and 2 partials ⚠️
provider/iterators.go 43.75% 11 Missing and 7 partials ⚠️
client/src/featureform/register.py 21.05% 13 Missing and 2 partials ⚠️
client/src/featureform/resources.py 28.57% 15 Missing ⚠️
provider/primary_table.go 41.66% 12 Missing and 2 partials ⚠️
coordinator/coordinator.go 42.85% 9 Missing and 3 partials ⚠️
provider/k8s.go 57.69% 10 Missing and 1 partial ⚠️
provider/offline.go 57.69% 11 Missing ⚠️
filestore/filepath.go 50.00% 3 Missing and 4 partials ⚠️
... and 7 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1428      +/-   ##
==========================================
- Coverage   57.34%   57.25%   -0.09%     
==========================================
  Files         195      194       -1     
  Lines       25692    25792     +100     
  Branches      854      864      +10     
==========================================
+ Hits        14732    14767      +35     
- Misses       9351     9420      +69     
+ Partials     1609     1605       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@epps epps had a problem deploying to Integration testing April 19, 2024 21:32 — with GitHub Actions Failure
@epps epps had a problem deploying to Integration testing April 19, 2024 22:10 — with GitHub Actions Failure
client/src/featureform/register.py Show resolved Hide resolved
client/src/featureform/register.py Show resolved Hide resolved
client/tests/test_spark_provider.py Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that all these changes will have to get merged in a more complex way in enterprise

coordinator/coordinator.go Outdated Show resolved Hide resolved
metadata/metadata_test.go Outdated Show resolved Hide resolved
metadata/proto/metadata.proto Outdated Show resolved Hide resolved
provider/iterators.go Outdated Show resolved Hide resolved
@epps epps had a problem deploying to Integration testing April 20, 2024 01:51 — with GitHub Actions Failure
@epps epps marked this pull request as ready for review April 20, 2024 01:53
@epps epps requested a review from simba-git April 20, 2024 01:53
client/src/featureform/resources.py Show resolved Hide resolved
filestore/filepath.go Show resolved Hide resolved
filestore/filepath.go Outdated Show resolved Hide resolved
metadata/proto/metadata.proto Outdated Show resolved Hide resolved
provider/bigquery.go Outdated Show resolved Hide resolved
provider/iterators.go Outdated Show resolved Hide resolved
provider/spark.go Show resolved Hide resolved
provider/spark.go Show resolved Hide resolved
provider/spark_test.go Show resolved Hide resolved
provider/sql.go Outdated Show resolved Hide resolved
@epps epps temporarily deployed to Integration testing April 20, 2024 03:46 — with GitHub Actions Inactive
@epps epps force-pushed the task/dfTransformation-refactor branch from 34f34bc to 1230a50 Compare April 20, 2024 03:50
@epps epps requested a review from simba-git April 20, 2024 03:50
@epps epps force-pushed the task/primary-table-refactor branch from 1df3c34 to 8a56041 Compare April 20, 2024 04:16
@epps epps temporarily deployed to Integration testing April 20, 2024 04:16 — with GitHub Actions Inactive
@epps epps changed the title completes initial backend refactor w/o changes to downstream methods Primary Table Registration Refactor Apr 20, 2024
@epps epps force-pushed the task/primary-table-refactor branch from 8a56041 to 32abbf7 Compare April 20, 2024 22:32
@epps epps temporarily deployed to Integration testing April 20, 2024 22:32 — with GitHub Actions Inactive
@epps epps temporarily deployed to Integration testing April 20, 2024 22:58 — with GitHub Actions Inactive
@epps epps force-pushed the task/dfTransformation-refactor branch 2 times, most recently from 30c3ea3 to 76fa46d Compare April 25, 2024 20:27
Base automatically changed from task/dfTransformation-refactor to main April 25, 2024 21:12
@epps epps force-pushed the task/primary-table-refactor branch 2 times, most recently from 38b98dc to c7f8f59 Compare April 25, 2024 21:22
@epps epps had a problem deploying to Integration testing April 25, 2024 21:22 — with GitHub Actions Failure
@epps epps force-pushed the task/primary-table-refactor branch from c7f8f59 to 485959a Compare April 25, 2024 21:43
@epps epps had a problem deploying to Integration testing April 25, 2024 21:43 — with GitHub Actions Failure
@epps epps force-pushed the task/primary-table-refactor branch from 485959a to aee67ac Compare April 25, 2024 22:32
@epps epps had a problem deploying to Integration testing April 25, 2024 22:32 — with GitHub Actions Failure
@epps epps force-pushed the task/primary-table-refactor branch from aee67ac to b4a5a9f Compare April 25, 2024 23:21
@epps epps temporarily deployed to Integration testing April 25, 2024 23:21 — with GitHub Actions Inactive
@epps epps temporarily deployed to Integration testing April 26, 2024 17:54 — with GitHub Actions Inactive
@epps epps force-pushed the task/primary-table-refactor branch from fd7ed47 to 27f285b Compare April 30, 2024 03:52
@epps epps had a problem deploying to Integration testing April 30, 2024 03:57 — with GitHub Actions Failure
@epps epps force-pushed the task/primary-table-refactor branch from b0c65b8 to 0d4cd80 Compare April 30, 2024 05:00
@epps epps temporarily deployed to Integration testing April 30, 2024 05:00 — with GitHub Actions Inactive
@epps epps force-pushed the task/primary-table-refactor branch from 0d4cd80 to 67e723b Compare April 30, 2024 12:50
@epps epps had a problem deploying to Integration testing April 30, 2024 12:50 — with GitHub Actions Failure
@epps epps temporarily deployed to Integration testing April 30, 2024 14:36 — with GitHub Actions Inactive
@epps epps temporarily deployed to Integration testing April 30, 2024 17:07 — with GitHub Actions Inactive
@epps epps had a problem deploying to Integration testing April 30, 2024 18:26 — with GitHub Actions Failure
@sdreyer sdreyer temporarily deployed to Integration testing April 30, 2024 21:00 — with GitHub Actions Inactive
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.

None yet

3 participants