feature: Add v2 SerDe compatability #1735

bveeramani · 2020-07-22T00:53:08Z

Issue #, if available: #1694

Description of changes:

Changed ASTTransformer implementation such that a modified node, None, or a new node can be returned (previously only a modified node would be returned).
Update all Modifiers to comply with new ASTTransformer implementation
Added serde module that contains classes for upgrading SerDe-related code.

Testing done:

Added test_serde

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 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 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.

sagemaker-bot · 2020-07-22T01:01:07Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-22T01:06:35Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-22T01:07:08Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-22T01:20:47Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-22T01:24:45Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-22T01:25:14Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-22T01:35:08Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-22T01:40:26Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-22T01:40:43Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-22T02:00:00Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-22T16:42:48Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-22T17:02:05Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-22T17:05:55Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-22T17:15:44Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-22T21:41:48Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-22T22:05:49Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-22T22:06:19Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-22T22:19:05Z

AWS CodeBuild CI Report

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

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

src/sagemaker/cli/compatibility/v2/modifiers/serde.py

metrizable

looks good so far! some small comments/requests

src/sagemaker/cli/compatibility/v2/modifiers/serde.py

Co-authored-by: Lauren Yu <[email protected]>

sagemaker-bot · 2020-07-24T20:06:30Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-24T20:06:31Z

AWS CodeBuild CI Report

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

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

Co-authored-by: Eric Johnson <[email protected]>

sagemaker-bot · 2020-07-24T20:13:33Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-24T22:16:58Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-24T22:33:13Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-24T22:47:52Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-24T23:17:22Z

AWS CodeBuild CI Report

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

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

src/sagemaker/cli/compatibility/v2/ast_transformer.py

src/sagemaker/cli/compatibility/v2/modifiers/serde.py

laurenyu · 2020-07-27T16:55:02Z

src/sagemaker/cli/compatibility/v2/modifiers/serde.py

+        self.class_names = class_names
+        self.import_node = import_node


I'm not sure if one way provides any benefit over the other, but you could also make these abstract property methods

I thought about it. I can't really decide which is better, so I'm leaving it as-is.

src/sagemaker/cli/compatibility/v2/modifiers/serde.py

sagemaker-bot · 2020-07-27T20:42:06Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-27T20:48:02Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-27T20:48:07Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-27T20:49:12Z

AWS CodeBuild CI Report

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

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

laurenyu

lgtm. pinging @metrizable since he requested changes on it earlier.

sagemaker-bot · 2020-07-27T21:10:45Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-27T21:11:41Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-27T21:19:12Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-27T21:22:27Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-27T21:22:54Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-27T21:22:59Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-27T22:06:43Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-28T16:12:15Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-28T16:31:10Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-28T16:46:50Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-07-28T16:49:33Z

AWS CodeBuild CI Report

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

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

metrizable

some small comments, addressable in a follow-up

metrizable · 2020-07-28T17:52:38Z

src/sagemaker/cli/compatibility/v2/modifiers/serde.py

+                the ``sagemaker.amazon.common`` module.
+        """
+        return node.module == "sagemaker.amazon.common" and any(
+            [alias.name in OLD_AMAZON_CLASS_NAMES for alias in node.names]


remove [ as unneeded as generator expression is fine

Suggested change

[alias.name in OLD_AMAZON_CLASS_NAMES for alias in node.names]

alias.name in OLD_AMAZON_CLASS_NAMES for alias in node.names

metrizable · 2020-07-28T17:56:22Z

src/sagemaker/cli/compatibility/v2/modifiers/serde.py

+    """
+
+    def __init__(self):
+        # Amazon SerDe are not defined in the sagemaker.serializers module.


nit: just make this a docstring?

metrizable · 2020-07-28T17:56:38Z

src/sagemaker/cli/compatibility/v2/modifiers/serde.py

+    """
+
+    def __init__(self):
+        # Amazon SerDe are not defined in the sagemaker.serializers module.


nit: just make this a docstring?

Balaji Veeramani added 3 commits July 21, 2020 19:50

Add SerDe compatability

cbcc622

Fix typo

2009c6a

Add test arguments

b8d6c07

Fix style issue

df021c9

Update v2.rst

6eab494

laurenyu reviewed Jul 23, 2020

View reviewed changes

metrizable suggested changes Jul 23, 2020

View reviewed changes

Update src/sagemaker/cli/compatibility/v2/modifiers/serde.py

39d1699

Co-authored-by: Lauren Yu <[email protected]>

Update src/sagemaker/cli/compatibility/v2/modifiers/serde.py

b9ce378

Co-authored-by: Eric Johnson <[email protected]>

laurenyu reviewed Jul 27, 2020

View reviewed changes

Balaji Veeramani and others added 3 commits July 27, 2020 15:39

Appease review comments

a309b62

Merge branch 'zwei' into serde-compatability

a677026

Update ast_transformer.py

5737872

laurenyu approved these changes Jul 27, 2020

View reviewed changes

bveeramani requested a review from metrizable July 27, 2020 21:36

Merge branch 'zwei' into serde-compatability

2f1c4cd

metrizable approved these changes Jul 28, 2020

View reviewed changes

bveeramani merged commit ea9fc4d into aws:zwei Jul 28, 2020

bveeramani deleted the serde-compatability branch July 28, 2020 18:08

bveeramani mentioned this pull request Jul 28, 2020

change: Improve code style of SerDe compatibility #1764

Merged

7 tasks

		self.class_names = class_names
		self.import_node = import_node

	[alias.name in OLD_AMAZON_CLASS_NAMES for alias in node.names]
	alias.name in OLD_AMAZON_CLASS_NAMES for alias in node.names

feature: Add v2 SerDe compatability #1735

feature: Add v2 SerDe compatability #1735

Conversation

bveeramani commented Jul 22, 2020

Merge Checklist

General

Tests

sagemaker-bot commented Jul 22, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 22, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 22, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 22, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 22, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 22, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 22, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 22, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 22, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 22, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 22, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 22, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 22, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 22, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 22, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 22, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 22, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 22, 2020

AWS CodeBuild CI Report

metrizable left a comment

Choose a reason for hiding this comment

sagemaker-bot commented Jul 24, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 24, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 24, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 24, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 24, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 24, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 24, 2020

AWS CodeBuild CI Report

laurenyu Jul 27, 2020

Choose a reason for hiding this comment

bveeramani Jul 27, 2020 • edited Loading

Choose a reason for hiding this comment

sagemaker-bot commented Jul 27, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 27, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 27, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 27, 2020

AWS CodeBuild CI Report

laurenyu left a comment

Choose a reason for hiding this comment

sagemaker-bot commented Jul 27, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 27, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 27, 2020

AWS CodeBuild CI Report

sagemaker-bot commented Jul 27, 2020

bveeramani Jul 27, 2020 •

edited

Loading