sagemaker-python-sdk
sagemaker-python-sdk copied to clipboard
Feature Store methods are broken
Describe the bug
When calling FeatureGroup.create (and other) method the SDK fails with the following exception:
Traceback (most recent call last):
File "test.py", line 15, in <module>
fg.create(
File "/home/guillem/miniconda3/envs/jk-nb/lib/python3.8/site-packages/sagemaker/feature_store/feature_group.py", line 521, in create
return self.sagemaker_session.create_feature_group(**create_feature_store_args)
TypeError: create_feature_group() missing 1 required positional argument: 'self'
To reproduce
import sagemaker
from sagemaker.feature_store.feature_group import FeatureDefinition, FeatureGroup, FeatureTypeEnum
sagemaker_sess = sagemaker.Session()
fds = [
FeatureDefinition(feature_name="label", feature_type=FeatureTypeEnum.INTEGRAL),
FeatureDefinition(feature_name="test", feature_type=FeatureTypeEnum.INTEGRAL),
FeatureDefinition(feature_name="message_id", feature_type=FeatureTypeEnum.STRING),
FeatureDefinition(feature_name="date", feature_type=FeatureTypeEnum.STRING),
]
fg = FeatureGroup(name="test-fs", feature_definitions=fds)
fg.create(
f"s3://{sagemaker_sess.default_bucket()}/test-store",
record_identifier_name="message_id",
event_time_feature_name="date",
role_arn=sagemaker.get_execution_role(sagemaker_sess),
)
System information A description of your system. Please provide:
- SageMaker Python SDK version: 2.88.1
- Python version: Python 3.8.12
- CPU or GPU: CPU
- Custom Docker image (Y/N): N
The SageMaker session needs to be passed in during init.
This works for me:
...
fg = FeatureGroup(name="test-fs", feature_definitions=fds, sagemaker_session=sagemaker_sess)
...
But then the sagemaker session shouldn’t be optional right?
I don't think so. The docs do not say it is optional.
The default value is the class sagemaker.session.Session, which is getting triggered in your example (I ran into the same a couple hours ago).
Right you are @setu4993 .
The offending code is here,
@attr.s
class FeatureGroup:
name: str = attr.ib(factory=str) # Though unrelated to this issue, we could also add a validator here
sagemaker_session: Session = attr.ib(default=Session) # Default is Session class, not an instance of it.
feature_definitions: Sequence[FeatureDefinition] = attr.ib(factory=list)
and should be updated to something like,
@attr.s
class FeatureGroup:
name: str = attr.ib(factory=str, validator=min_len(1)) # validator will ensure name as provided
sagemaker_session: Session = attr.ib(factory=Session) # Now it would create an instance of sagemaker Session
feature_definitions: Sequence[FeatureDefinition] = attr.ib(factory=list)
Fixed by https://github.com/aws/sagemaker-python-sdk/pull/3667
Thank you for reporting