-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Feature processor v3 #5565
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
Merged
Merged
Feature processor v3 #5565
Changes from 19 commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
56eef2d
Feature store v3 (#5490)
Aditi2424 d5fd783
feat: feature_processor v3
bhhalim e91d8d2
integ tests
bhhalim 4079a5d
fix
bhhalim e6721ce
chore(docs): Add API docs
bhhalim 611f577
fix: Fix flaky integ tests
bhhalim 6f4f490
fix diff
bhhalim 09abb01
chore: rename parameter + cleanup comments
bhhalim a841d7d
Merge branch 'master' into feature-processor-v3
mollyheamazon aa654e4
Feature store v3 (#5490)
Aditi2424 7cdd077
add pyspark to test deps
bhhalim 101824f
add test deps
bhhalim 1e2b5fe
fix unit test deps
bhhalim bbf1a3f
pin setuptools<82 for feature-processor and unit tests
bhhalim 5ca86dc
Set JAVA_HOME for integ tests which requires java
bhhalim beb078d
fix spark session bug
bhhalim 7a659af
fix(feature-processor): Fix Spark session config and Ivy cache race c…
bhhalim 59b68e8
revert previous change + create different ivy cache per test to fix c…
bhhalim fa59f5e
Merge branch 'master' into feature-processor-v3
BassemHalim a1b1bc3
revert changes to sagemaker-core
bhhalim 3cd4686
refactor(feature-processor): Migrate to FeatureGroup resource API
bhhalim 39f1be2
add `build` to test_requirements
bhhalim 4797633
add upper bounds for test dependencies
bhhalim e2e179f
move feature-processor config to sagemaker-mlops optional deps
bhhalim File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 is this needed ?
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.
feature-processor used to require an extra pip install
pip install sagemaker[feature-processor]this is because it requires extra dependencies that don't make sense to require all sagemaker users to install docsV2 requirements: https://github.com/aws/sagemaker-python-sdk/blob/master-v2/requirements/extras/feature-processor_requirements.txt
The extra setuptools<82 is because setuptools removed a module named
pkg_resourceshttps://setuptools.pypa.io/en/latest/history.html#v82-0-0There 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.
feature-processor is a functionality within sagemaker-mlops .
From our tenet , we are creating namespaces for higher level flows such as training , inference , mlops.
Can this be within sagemaker-mlops
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.
it can but do we want everyone doing
pip install sagemaker-mlopsto install pyspark?Also if we move it to sagemaker-mlops we would need to update all docs that reference installing
sagemaker[feature-processor]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.
I think that is fine if they install pyspark .
We are providing the package/abstraction at the mlops level.
Also docs have to be updated anyways since this is a major version change with changes to imports/installations and inputs