Add python_version for glue jobs #9409

mjuarez · 2019-07-18T19:18:58Z

Problem

Unable to specify the version of Python to use with pythonshell glue jobs despite supported in API

Solution

Add in ability to specify the python version with the JobCommand as mentioned in AWS SDK

Signed-off-by: Mike Juarez mikejuarez@gmail.com

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Relates #9524

Release note for CHANGELOG:

Added ability to specify python version for pythonshell in glue jobs

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccXXX'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -parallel 20 -run=TestAccXXX -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
testing: warning: no tests to run
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	0.045s [no tests to run]

Signed-off-by: Mike Juarez <mikejuarez@gmail.com>

nywilken

Thanks for the quick fix.

nywilken · 2019-07-18T22:30:33Z

Sorry I saw the document change and approved without looking at the rest of the PR.

nywilken · 2019-07-18T22:31:59Z

aws/resource_aws_glue_job.go

@@ -44,6 +44,10 @@ func resourceAwsGlueJob() *schema.Resource {
 							Type:     schema.TypeString,
 							Required: true,
 						},
+						"python_version": {
+							Type:     schema.TypeString,
+							Optional: true,


Let’s add a validation check for this attribute.

ValidateFunc: validation.StringInSlice([]string{"2","3"}, true),

nywilken

make testacc TESTARGS=“-run=TestAccAWSGlueJob_PythonShell“ is what you need to run the modified test case. See our CONTRIBUTING for more information on writing tests.

nywilken

Please see my comments above.

Signed-off-by: Michael Juarez <mikejuarez@gmail.com>

mjuarez · 2019-07-22T23:22:32Z

@nywilken Thanks for the guidance. Here's the latest from acceptance tests.

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSGlueJob_PythonShell'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSGlueJob_PythonShell -timeout 120m
=== RUN   TestAccAWSGlueJob_PythonShell
=== PAUSE TestAccAWSGlueJob_PythonShell
=== CONT  TestAccAWSGlueJob_PythonShell
--- PASS: TestAccAWSGlueJob_PythonShell (28.53s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	28.564s

mjuarez · 2019-07-24T18:43:38Z

@nywilken Looks like Travis failed due to OutOfMemory errors

nywilken

@mjuarez this looks good. I did some additional testing and found some quirks with the support for the various Python Versions. I left some hefty comments mostly for background context with code suggestions to address the quirks. Please let me know if you have any questions.

nywilken · 2019-07-31T17:58:18Z

aws/resource_aws_glue_job.go

@@ -44,6 +44,11 @@ func resourceAwsGlueJob() *schema.Resource {
 							Type:     schema.TypeString,
 							Required: true,
 						},
+						"python_version": {
+							Type:         schema.TypeString,
+							Optional:     true,


Looks like there are some constraints around the python version. If using Glue Version 0.9 which is what is currently being defined within this resource the default Python Version will be 2, but when using Glue Version 1.0, which we currently don't support, the default value will be 3. So we should add Computed: true as there is no good default at this time. When we add support for "glue_version" #9524 we can revisit if need be.

nywilken · 2019-07-31T18:01:58Z

aws/resource_aws_glue_job.go

@@ -331,6 +336,7 @@ func expandGlueJobCommand(l []interface{}) *glue.JobCommand {
 	jobCommand := &glue.JobCommand{
 		Name:           aws.String(m["name"].(string)),
 		ScriptLocation: aws.String(m["script_location"].(string)),
+		PythonVersion:  aws.String(m["python_version"].(string)),


The API will error if PythonVersion is not a valid version so we should not send a value if no "python_version" was defined in the Terraform configuration.

Suggested change

PythonVersion: aws.String(m["python_version"].(string)),

}

if v, ok := m["python_version"].(string); ok && v != "" {

jobCommand.PythonVersion = aws.String(v)

}

nywilken · 2019-07-31T18:22:30Z

aws/resource_aws_glue_job_test.go

@@ -729,6 +730,7 @@ resource "aws_glue_job" "test" {
  command {
    name            = "pythonshell"
    script_location = "testscriptlocation"
+    python_version =  "3"


Let's remove the version from the configuration to validate that setting no python_version works as expected.

Suggested change

python_version = "3"

nywilken · 2019-07-31T18:23:29Z

aws/resource_aws_glue_job_test.go

@@ -397,6 +397,7 @@ func TestAccAWSGlueJob_PythonShell(t *testing.T) {
 					resource.TestCheckResourceAttr(resourceName, "max_capacity", "0.0625"),
 					resource.TestCheckResourceAttr(resourceName, "command.#", "1"),
 					resource.TestCheckResourceAttr(resourceName, "command.0.script_location", "testscriptlocation"),
+					resource.TestCheckResourceAttr(resourceName, "command.0.python_version", "3"),


When no python_version is specified the default response from the API will be version 2. Once support is added for glue version 1.0 the default value will be 3.

Suggested change

resource.TestCheckResourceAttr(resourceName, "command.0.python_version", "3"),

resource.TestCheckResourceAttr(resourceName, "command.0.python_version", "2"),

Let's update this test to account for the three PythonVersion use cases (i.e Not Set, Set to 2, Set to 3).

Start by adding the following test configuration under the existing testAccAWSGlueJobConfig_PythonShell function

func testAccAWSGlueJobConfig_PythonShellWithVersion(rName, pythonVersion string) string { return fmt.Sprintf(` %s resource "aws_glue_job" "test" { name = %q role_arn = "${aws_iam_role.test.arn}" max_capacity = 0.0625 command { name = "pythonshell" script_location = "testscriptlocation" python_version = %q } depends_on = ["aws_iam_role_policy_attachment.test"] } `, testAccAWSGlueJobConfig_Base(rName), rName, pythonVersion) }

Then update the test to look like the following:

func TestAccAWSGlueJob_PythonShell(t *testing.T) { var job glue.Job rName := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(5)) resourceName := "aws_glue_job.test" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckAWSGlueJobDestroy, Steps: []resource.TestStep{ { Config: testAccAWSGlueJobConfig_PythonShell(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSGlueJobExists(resourceName, &job), resource.TestCheckResourceAttr(resourceName, "max_capacity", "0.0625"), resource.TestCheckResourceAttr(resourceName, "command.#", "1"), resource.TestCheckResourceAttr(resourceName, "command.0.script_location", "testscriptlocation"), resource.TestCheckResourceAttr(resourceName, "command.0.python_version", "2"), resource.TestCheckResourceAttr(resourceName, "command.0.name", "pythonshell"), ), }, { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, }, { Config: testAccAWSGlueJobConfig_PythonShellWithVersion(rName, "2"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSGlueJobExists(resourceName, &job), resource.TestCheckResourceAttr(resourceName, "max_capacity", "0.0625"), resource.TestCheckResourceAttr(resourceName, "command.#", "1"), resource.TestCheckResourceAttr(resourceName, "command.0.script_location", "testscriptlocation"), resource.TestCheckResourceAttr(resourceName, "command.0.python_version", "2"), resource.TestCheckResourceAttr(resourceName, "command.0.name", "pythonshell"), ), }, { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, }, { Config: testAccAWSGlueJobConfig_PythonShellWithVersion(rName, "3"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSGlueJobExists(resourceName, &job), resource.TestCheckResourceAttr(resourceName, "max_capacity", "0.0625"), resource.TestCheckResourceAttr(resourceName, "command.#", "1"), resource.TestCheckResourceAttr(resourceName, "command.0.script_location", "testscriptlocation"), resource.TestCheckResourceAttr(resourceName, "command.0.python_version", "3"), resource.TestCheckResourceAttr(resourceName, "command.0.name", "pythonshell"), ), }, { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, }, }, }) }```

nywilken · 2019-08-01T17:36:19Z

This pull request appears to be related to #9524, so I have edited the pull request description to denote the issue reference.

Signed-off-by: Michael Juarez <mikejuarez@gmail.com>

mjuarez · 2019-08-06T22:51:17Z

@nywilken Thanks for the guidance. I've updated with your suggestions. Not sure how to get this to fit with #9524

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSGlueJob_PythonShell -timeout 120m
=== RUN   TestAccAWSGlueJob_PythonShell
=== PAUSE TestAccAWSGlueJob_PythonShell
=== CONT  TestAccAWSGlueJob_PythonShell
--- PASS: TestAccAWSGlueJob_PythonShell (68.30s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	(cached)

stephencsmall · 2019-08-29T17:06:07Z

Is there anything blocking this from being merged? Would love to have this for my current project, willing to help get it done if I can.

nywilken

@mjuarez apologies for the delays here. I rebased your changes onto master and all is working as expected. Apologies again. Thank you for the contribution and time.

--- PASS: TestAccAWSGlueJob_PythonShell (51.98s)

Add python_version for glue jobs

nywilken · 2019-09-18T20:42:07Z

The enhancement to the aws_glue_job resource has been merged and will be released with version 2.29.0 of the Terraform AWS provider.

kabeersvohra · 2019-09-20T09:15:11Z

@nywilken any idea when 2.29.0 is due to be released? cheers

nywilken · 2019-09-20T15:07:26Z

@kabeersvohra we generally release every Thursday, but we ran into a slight issue in testing one of our recently modified resources (unrelated to this resource). We are looking into the issue and should have a release out by today. If things should change I'll follow up on this thread. Thanks for your patience with us in getting the release out.

ghost · 2019-09-20T18:00:11Z

This has been released in version 2.29.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

kabeersvohra · 2019-09-24T10:15:02Z

I have found a slight bug in setting the python version in this PR. When I have a glue job which is set to Spark 2.2, Python 2 (Glue version 0.9) in the console and I set the python_version to 3 in terraform, after applying, the delta is gone but the job glue version is blank in the console. This issue does not occur when the job is set to Spark 2.4, Python 2 (Glue version 1.0) in the console, I am guessing it is to do with a mismatch of the python version and glue version which is not set in this PR. Thanks

mjuarez · 2019-09-24T15:48:53Z

@kabeersvohra I'm wondering if this is related to #9524

kabeersvohra · 2019-09-25T09:14:49Z

@mjuarez yes, and a PR to solve this has just been created: #10237

ghost · 2019-11-01T15:06:35Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

Add python_version for glue jobs

0c5176a

Signed-off-by: Mike Juarez <mikejuarez@gmail.com>

mjuarez requested a review from a team July 18, 2019 19:18

fix mispelling

cbe1e4b

nywilken approved these changes Jul 18, 2019

View reviewed changes

nywilken reviewed Jul 18, 2019

View reviewed changes

nywilken suggested changes Jul 18, 2019

View reviewed changes

add validatefunc and properly flatten jobcommand

07861e1

Signed-off-by: Michael Juarez <mikejuarez@gmail.com>

mjuarez closed this Jul 29, 2019

mjuarez reopened this Jul 29, 2019

nywilken self-assigned this Jul 30, 2019

nywilken reviewed Jul 31, 2019

View reviewed changes

nywilken mentioned this pull request Jul 31, 2019

Add ability for aws_glue_job to use Python 3 and glue version 1.0 #9524

Closed

nywilken added the enhancement Requests to existing resources that expand the functionality or scope. label Aug 1, 2019

add in new test cases and computed python versions

a6bf6ed

Signed-off-by: Michael Juarez <mikejuarez@gmail.com>

ghost added size/S Managed by automation to categorize the size of a PR. and removed size/XS Managed by automation to categorize the size of a PR. labels Aug 6, 2019

nywilken added this to the v2.29.0 milestone Sep 18, 2019

nywilken approved these changes Sep 18, 2019

View reviewed changes

nywilken added a commit that referenced this pull request Sep 18, 2019

Merge pull request #9409 from mjuarez/glue_python_version

54f85b6

Add python_version for glue jobs

nywilken closed this Sep 18, 2019

nywilken reopened this Sep 18, 2019

nywilken added a commit that referenced this pull request Sep 18, 2019

update CHANGELOG for #9409

a02170e

nywilken closed this Sep 18, 2019

ghost locked and limited conversation to collaborators Nov 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add python_version for glue jobs #9409

Add python_version for glue jobs #9409

mjuarez commented Jul 18, 2019 •

edited by nywilken

Loading

nywilken left a comment

nywilken commented Jul 18, 2019

nywilken Jul 18, 2019 •

edited

Loading

nywilken left a comment •

edited

Loading

nywilken left a comment

mjuarez commented Jul 22, 2019

mjuarez commented Jul 24, 2019

nywilken left a comment

nywilken Jul 31, 2019

nywilken Jul 31, 2019

nywilken Jul 31, 2019

nywilken Jul 31, 2019

nywilken Jul 31, 2019

nywilken commented Aug 1, 2019

mjuarez commented Aug 6, 2019

stephencsmall commented Aug 29, 2019

nywilken left a comment

nywilken commented Sep 18, 2019

kabeersvohra commented Sep 20, 2019

nywilken commented Sep 20, 2019 •

edited

Loading

ghost commented Sep 20, 2019

kabeersvohra commented Sep 24, 2019

mjuarez commented Sep 24, 2019

kabeersvohra commented Sep 25, 2019

ghost commented Nov 1, 2019

-		PythonVersion:  aws.String(m["python_version"].(string)),
+}
+if v, ok := m["python_version"].(string); ok &&  v != "" {
+  jobCommand.PythonVersion = aws.String(v)
+ }

	resource.TestCheckResourceAttr(resourceName, "command.0.python_version", "3"),
	resource.TestCheckResourceAttr(resourceName, "command.0.python_version", "2"),

Add python_version for glue jobs #9409

Add python_version for glue jobs #9409

Conversation

mjuarez commented Jul 18, 2019 • edited by nywilken Loading

Problem

Solution

Community Note

nywilken left a comment

Choose a reason for hiding this comment

nywilken commented Jul 18, 2019

nywilken Jul 18, 2019 • edited Loading

Choose a reason for hiding this comment

nywilken left a comment • edited Loading

Choose a reason for hiding this comment

nywilken left a comment

Choose a reason for hiding this comment

mjuarez commented Jul 22, 2019

mjuarez commented Jul 24, 2019

nywilken left a comment

Choose a reason for hiding this comment

nywilken Jul 31, 2019

Choose a reason for hiding this comment

nywilken Jul 31, 2019

Choose a reason for hiding this comment

nywilken Jul 31, 2019

Choose a reason for hiding this comment

nywilken Jul 31, 2019

Choose a reason for hiding this comment

nywilken Jul 31, 2019

Choose a reason for hiding this comment

nywilken commented Aug 1, 2019

mjuarez commented Aug 6, 2019

stephencsmall commented Aug 29, 2019

nywilken left a comment

Choose a reason for hiding this comment

nywilken commented Sep 18, 2019

kabeersvohra commented Sep 20, 2019

nywilken commented Sep 20, 2019 • edited Loading

ghost commented Sep 20, 2019

kabeersvohra commented Sep 24, 2019

mjuarez commented Sep 24, 2019

kabeersvohra commented Sep 25, 2019

ghost commented Nov 1, 2019

mjuarez commented Jul 18, 2019 •

edited by nywilken

Loading

nywilken Jul 18, 2019 •

edited

Loading

nywilken left a comment •

edited

Loading

nywilken commented Sep 20, 2019 •

edited

Loading