feat(glue): Added value to PythonVersion enum

[meve]

PR to fix #21568. Extended the PythonVersion enum to include 3.9, as it already seems to be supported everywhere (CloudFormation, SDK). #21568 (comment)

---

All Submissions:

• Have you followed the guidelines in our Contributing guide?

Adding new Unconventional Dependencies:

• This PR adds new unconventional dependencies following the process described here

New Features

• Have you added the new feature to an integration test?
  • Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

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

[gitpod-io]

[tmokmss]

I think we should add a validation here since3.9 should only work for pythonshell jobs

aws-cdk/packages/@aws-cdk/aws-glue/lib/job-executable.ts

Lines 285 to 293 in 2502321

	private constructor(config: JobExecutableConfig) {
	if (JobType.PYTHON_SHELL === config.type) {
	if (config.language !== JobLanguage.PYTHON) {
	throw new Error('Python shell requires the language to be set to Python');
	}
	if ([GlueVersion.V0_9, GlueVersion.V2_0, GlueVersion.V3_0].includes(config.glueVersion)) {
	throw new Error(`Specified GlueVersion ${config.glueVersion.name} does not support Python Shell`);
	}
	}

[meve]

I have added a validation for the condition you suggested. However, I would like to point out that some of the other validations seem to be a bit off. For instance: it should be possible to create a pythonshell job with any GlueVersion. However, it is not possible to do this through CDK.

[TheRealAmazonKendra]

I have added a validation for the condition you suggested. However, I would like to point out that some of the other validations seem to be a bit off. For instance: it should be possible to create a pythonshell job with any GlueVersion. However, it is not possible to do this through CDK.

Can you expand upon this and which other validations are off? We should make sure we have a ticket to get those fixed.

[meve]

So for instance this one:

aws-cdk/packages/@aws-cdk/aws-glue/lib/job-executable.ts

Lines 290 to 292 in 5cc0d35

	if ([GlueVersion.V0_9, GlueVersion.V2_0, GlueVersion.V3_0].includes(config.glueVersion)) {
	throw new Error(`Specified GlueVersion ${config.glueVersion.name} does not support Python Shell`);
	}

I actually thought of fixing it also in this PR, but I also figured I might risk it being rejected. But again using an escape hatch, I just tried to create a pythonshell Job using any GlueVersion (0.9, 1.0, 2.0, 3.0). All succeeded. It seems that the GlueVersion is ignored completely by the API when a pythonshell job is created.

The CloudFormation documentation [0] says:

Jobs that are created without specifying a Glue version default to Glue 0.9.

But still that seems irrelevant for pythonshell jobs, and that makes me think that the validation does not really make sense, as it is completely irrelevant what GlueVersion is passed when creating a pythonshell. Also, when I perform a GetJob call, the GlueVersion is returned nowhere if it's not specified when creating it.

I hope this makes sense.

[0] https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-glue-job.html#cfn-glue-job-glueversion

[tmokmss]

@meve So the main problem seems to be glueVersion property is currently taken as required while in reality it is unnecessary for python shell jobs.
How about fix it in another PR? For now I believe the only validation we need is for python version.

[meve]

@tmokmss I agree that is the problem, and I also agree with creating another PR for this. I will create one over the next few days.

What is not clear to me, is what now is left to do for the current PR.

[tmokmss]

@meve
I guess you should fix CI before the review, which is currently failing. How about updating the sample code on README.md to use python 3.9?

Error: Features must contain a change to a README file

Also you can refer to this document about contributing to CDK :)

[meve]

@tmokmss Something else. So in CDK and the API, PythonVersion.TWO is still supported. The console however seems to have dropped support for it (e.g., it cannot display the value anymore). Deploying a job still works though, and I still can run it:

import sys

print(sys.version)

gives:

2.7.18 (default, Feb 18 2021, 06:10:44) [GCC 4.8.5 20150623 (Red Hat 4.8.5-28)]

See also here.

The point I am trying to make is that I decided to add it the way I did, is to at least try to point out the use of PythonVersion.TWO should not be encouraged.

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: a18facc
• Result: SUCCEEDED
• Build Logs (available for 30 days)

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

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).