search

aws-samples / amazon-sagemaker-feature-store-end-to-end-workshop

MIT No Attribution
124 stars 49 forks source link

Update sagemaker python sdk version and update SageMaker Studio graphics for new UI. #40

Open jld23 opened 1 year ago

jld23 commented 1 year ago

Issue #, if available:

Description of changes: The current notebooks are based on sagemaker sdk 2.48 which is 18 months old and ~85 versions behind. I updated the sagemaker version check from:

if sagemaker.__version__ < '2.48.1':
    subprocess.check_call([sys.executable, '-m', 'pip', 'install', 'sagemaker==2.48.1'])
    importlib.reload(sagemaker)

to this because the prior version did a string comparison so 132 < 48.

current_sagemaker_release = '2.132.0'
curr_major, curr_minor, curr_patch = current_sagemaker_release.split('.')
sagemaker_version = sagemaker.__version__
sm_major, sm_minor, sm_patch = sagemaker_version.split('.')
if int(sm_major) < int(curr_major) or int(sm_minor) < int(curr_minor):
    subprocess.check_call([sys.executable, '-m', 'pip', 'install', f'sagemaker=={current_sagemaker_release}'])
    importlib.reload(sagemaker)

I also updated the screen shots and instructions in the ./images folder to reflect the UI changes to SageMaker Studio in November 2022.

I ran all the notebooks with the updated sagemaker python SDK (v2.132.0) and all the notebooks ran successfully except for m6_nb1 for which I'll open a separate issue.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

jld23 commented 1 year ago

I reviewed the diffs caused by the latest push. All are markdown cell id values that aren't visible to the user. I'm happy to update the conflict but I'd like to do it once so if you can give me. time line or indication if this pull request will be accepted I would appreciate the communication.

markproy commented 1 year ago

yeah, I'd rather not commit 33 file changes unnecessarily. if you can get it back down to a simple handful of changes, we can get it approved in short order. no rush. once you have it ready, we can probably get someone to get through it w/in a week most likely.

jld23 commented 1 year ago

@markproy I updated the commit to just the 15 images and 8 notebooks that needed updates. There are no longer any conflicts with the base branch.

markproy commented 1 year ago

Thanks for streamlining the changes. I really appreciate the updated images! The 8 nb's that need to change still have lots of diffs for some reason. Why so many changes for each nb?

jld23 commented 1 year ago

It looks like the changes are in metadata of the notebook. I used python 3.8 and the previous notebooks were 3.7. Also, it appears Sagemaker is writing a bunch of metadata about instance types.