Create a default SageMaker Session inside FeatureGroup class #3667

psnilesh · 2023-02-19T14:39:42Z

Issue

Description

Sagemaker Session in FeatureGroup class is not initialized properly if the input is None. It is being set to Session class instead of an instance of it (i.e session = Session instead of sesssion = Session()).

Problems created by this incorrect assignment is explained in linked issue.

Testing

Installed the SDK in virutalenv, created a FeatureGroup without passing a Session and invoked describe().

from sagemaker.feature_store.feature_group import FeatureGroup

f = FeatureGroup(
    name = 'test-fg'
)

print(f.describe())

Output

botocore.errorfactory.ResourceNotFound: An error occurred (ResourceNotFound) when calling the DescribeFeatureGroup operation: Resource Not Found: Amazon SageMaker can't find a FeatureGroup with name test-fg

Above error implies the session was instantiated correctly and the service call went through.

Merge Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.

General

I have read the CONTRIBUTING doc
I certify that the changes I am introducing will be backward compatible, and I have discussed concerns about this, if any, with the Python SDK team
I used the commit message format described in CONTRIBUTING
I have passed the region in to all S3 and STS clients that I've initialized as part of this change.
I have updated any necessary documentation, including READMEs and API docs (if appropriate)

Tests

I have added tests that prove my fix is effective or that my feature works (if appropriate)
I have added unit and/or integration tests as appropriate to ensure backward compatibility of the changes
I have checked that my tests are not configured for a specific region or account (if appropriate)
I have used unique_name_from_base to create resource names in integ tests (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

claytonparnell

/bot run all

sagemaker-bot · 2023-02-20T17:56:55Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: c962081
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-02-20T18:02:40Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: c962081
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-02-20T18:09:53Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: c962081
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

codecov-commenter · 2023-02-20T18:20:01Z

Codecov Report

Merging #3667 (c962081) into master (dcca7b7) will decrease coverage by 0.95%.
The diff coverage is 100.00%.

❗ Current head c962081 differs from pull request most recent head 01dcbfd. Consider uploading reports for the commit 01dcbfd to get more accurate results

@@            Coverage Diff             @@
##           master    #3667      +/-   ##
==========================================
- Coverage   89.73%   88.79%   -0.95%     
==========================================
  Files         968      227     -741     
  Lines       90972    22454   -68518     
==========================================
- Hits        81636    19937   -61699     
+ Misses       9336     2517    -6819

Impacted Files	Coverage Δ
src/sagemaker/feature_store/feature_group.py	`74.56% <100.00%> (ø)`
...ython3.9/site-packages/sagemaker/workflow/steps.py
...python3.7/site-packages/sagemaker/sparkml/model.py
....10/site-packages/sagemaker/workflow/conditions.py
...-packages/sagemaker/workflow/parallelism_config.py
.../sagemaker/cli/compatibility/v2/ast_transformer.py
...x/py37/lib/python3.7/site-packages/sagemaker/s3.py
...python3.8/site-packages/sagemaker/xgboost/model.py
...thon3.7/site-packages/sagemaker/mxnet/estimator.py
...on3.10/site-packages/sagemaker/sklearn/defaults.py
... and 1186 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

sagemaker-bot · 2023-02-20T18:20:05Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: c962081
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-02-20T18:37:12Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: c962081
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

psnilesh · 2023-02-21T14:16:55Z

sagemaker-python-sdk-slow-tests

FAILED tests/integ/test_inference_pipeline.py::test_inference_pipeline_model_deploy_and_update_endpoint

sagemaker-python-sdk-pr

FAILED tests/integ/sagemaker/experiments/test_run.py::test_load_run_auto_pass_in_exp_config_to_job
FAILED tests/integ/sagemaker/experiments/test_run.py::test_run_from_processing_job_and_override_default_exp_config

Failures look unrelated to the changes in PR.

vikas0203

/bot run pr,slow-tests

sagemaker-bot · 2023-02-21T18:25:08Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: c962081
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-02-21T20:44:18Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: c962081
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

…provided

claytonparnell

/bot run slow-tests

sagemaker-bot · 2023-02-23T03:40:54Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: 01dcbfd
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-02-23T04:11:25Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: 01dcbfd
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-02-23T04:21:38Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 01dcbfd
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-02-23T04:25:41Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 01dcbfd
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-02-23T04:30:37Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: 01dcbfd
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2023-02-23T18:39:17Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: 01dcbfd
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

psnilesh · 2023-02-24T15:20:05Z

The latest failure seem to be in inference pipeline where feature store isn't in use.

vikas0203

/bot run slow-tests

sagemaker-bot · 2023-02-24T19:23:02Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: 01dcbfd
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Co-authored-by: Nilesh <[email protected]>

psnilesh requested a review from a team as a code owner February 19, 2023 14:39

psnilesh requested review from vikas0203 and removed request for a team February 19, 2023 14:39

claytonparnell assigned vikas0203 Feb 20, 2023

claytonparnell reviewed Feb 20, 2023

View reviewed changes

vikas0203 reviewed Feb 21, 2023

View reviewed changes

fix: create a default sagemaker Session in FeatureGroup class if not …

01dcbfd

…provided

claytonparnell force-pushed the master branch from c962081 to 01dcbfd Compare February 23, 2023 02:31

claytonparnell reviewed Feb 23, 2023

View reviewed changes

vikas0203 reviewed Feb 24, 2023

View reviewed changes

vikas0203 merged commit c35de7b into aws:master Feb 27, 2023

JoseJuan98 pushed a commit to JoseJuan98/sagemaker-python-sdk that referenced this pull request Mar 4, 2023

Create a default SageMaker Session inside FeatureGroup class (aws#3667)

9626d9b

Co-authored-by: Nilesh <[email protected]>

nmadan pushed a commit to nmadan/sagemaker-python-sdk that referenced this pull request Apr 18, 2023

Create a default SageMaker Session inside FeatureGroup class (aws#3667)

acdb58b

Co-authored-by: Nilesh <[email protected]>

martinRenou mentioned this pull request Sep 27, 2023

Feature Store methods are broken #3090

Closed

Create a default SageMaker Session inside FeatureGroup class #3667

Create a default SageMaker Session inside FeatureGroup class #3667

Uh oh!

Conversation

psnilesh commented Feb 19, 2023

Issue

Description

Testing

Merge Checklist

General

Tests

Uh oh!

claytonparnell left a comment

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Feb 20, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 20, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 20, 2023

AWS CodeBuild CI Report

Uh oh!

codecov-commenter commented Feb 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sagemaker-bot commented Feb 20, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 20, 2023

AWS CodeBuild CI Report

Uh oh!

psnilesh commented Feb 21, 2023

Uh oh!

vikas0203 left a comment

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Feb 21, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 21, 2023

AWS CodeBuild CI Report

Uh oh!

claytonparnell left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Feb 23, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 23, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 23, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 23, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 23, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 23, 2023

AWS CodeBuild CI Report

Uh oh!

psnilesh commented Feb 24, 2023

Uh oh!

vikas0203 left a comment

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Feb 24, 2023

AWS CodeBuild CI Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

codecov-commenter commented Feb 20, 2023 •

edited

Loading

claytonparnell left a comment •

edited

Loading