Skip to content

Conversation

pintaoz-aws
Copy link
Collaborator

@pintaoz-aws pintaoz-aws commented Sep 12, 2024

Issue #, if available:

Description of changes:

Add the content in track_local_pytorch_experiment.ipynb to integ test. Add Endpoint.invoke() and Endpoint.invoke_with_response_stream() to existing test. Fix some bugs found during testing.

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

@pintaoz-aws pintaoz-aws force-pushed the sample-notebook-integ-test branch from 764c500 to 06dc458 Compare September 13, 2024 00:15
@pintaoz-aws pintaoz-aws changed the title Add code example notebooks to integ tests Add example notebooks to integ tests Sep 13, 2024
"""
serialized_dict = {}
for k, v in value.items():
if serialize_result := serialize(v):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The previous condition will miss values like integer 0

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we do if serialize_result := serialize(v) is not None: ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated

@pintaoz-aws pintaoz-aws marked this pull request as ready for review September 13, 2024 00:28
Comment on lines +75 to +77
trial_component.disassociate_trail(trial_name=trial.trial_name)
trial_component.delete()
trial.delete()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we move this to SageMakerCleaner so all resources cleanup is managed in one place ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TrialComponent must disassociate with Trial first, and they have to be deleted in the order TrialComponent -> Trial -> Experiment. So they can not be deleted in the way of deleting the same type of resources together

"""
serialized_dict = {}
for k, v in value.items():
if serialize_result := serialize(v):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we do if serialize_result := serialize(v) is not None: ?


assert invoke_result.body

invoke_result = endpoint.invoke_with_response_stream(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Test seems to be failing .

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@pintaoz-aws pintaoz-aws force-pushed the sample-notebook-integ-test branch from 3519265 to abf5867 Compare September 13, 2024 23:17
@sagemaker-bot sagemaker-bot requested a review from a team as a code owner April 21, 2025 10:20
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.

2 participants