fix(glue): PythonRayExecutableProps has innaccurate properties
#28625
Conversation
github-actions
bot
added
bug
aws-cdk-automation
added
the
pr/needs-community-review
|
@lpizzinidev Thank you for looking! Could you please approve and merge this if it is okay? |
|
@moomindani Merging requires a core team member, I'm just a community reviewer 🙂 |
|
This PR has been in the MERGE CONFLICTS state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week. |
aws-cdk-automation
added
pr/needs-maintainer-review
kaizencc
changed the title
PythonRayExecutableProps has innaccurate properties
There was a problem hiding this comment.
hi @moomindani, I have a few concerns before we merge this PR
| * | ||
| * @default - no extra python files specified. | ||
| * | ||
| * @see `--s3-py-modules` in https://docs.aws.amazon.com/glue/latest/dg/author-job-ray-job-parameters.html |
There was a problem hiding this comment.
@see expects a url after the tag, and nothing else
There was a problem hiding this comment.
It's zipped utils.py which is used for this integration test. Since s3PythonModules accepts only Zip file, we need to have this.
I have verified that other packages in AWS CDK has similar zip files so I do not think this can be blocker.
| @@ -253,7 +269,7 @@ export interface PythonShellExecutableProps extends SharedJobExecutableProps, Py | |||
| /** | |||
| * Props for creating a Python Ray job executable | |||
| */ | |||
| export interface PythonRayExecutableProps extends SharedJobExecutableProps, PythonExecutableProps {} | |||
| export interface PythonRayExecutableProps extends SharedJobExecutableProps, RayExecutableProps {} | |||
There was a problem hiding this comment.
what happens to PythonExecutableProps? a) is it used elsewhere and b) we are not dropping support for any properties right?
There was a problem hiding this comment.
if we are dropping support for properties (as the attached issue suggests) it should have been called out in PR description as a BREAKING CHANGE.
There was a problem hiding this comment.
a) is it used elsewhere
Yes. There are three job types; Spark, Python shell, and Ray. PythonExecutableProps is used for the first two.
b) we are not dropping support for any properties right?
Right. We are not dropping any or working properties.
Only the property we are dropping is extraPythonFiles which was not working for Ray job. This is not breaking change.
aws-cdk-automation
removed
the
pr/needs-maintainer-review
mergify
bot
dismissed
kaizencc’s stale review
Pull request has been modified.
aws-cdk-automation
added
the
pr/needs-maintainer-review
|
@kaizencc Can you take a look and merge it if there are no further update needed? |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
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). |
Closes #28570.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license