Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

updates to prep demand scaffolding fn #6

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

rpmcginty
Copy link
Collaborator

@rpmcginty rpmcginty commented Jun 2, 2024

What's in this Change?

This contains updates to the prepare-demand-scaffolding lambda function.

  • adding option to specify file system configurations for shared and scratch directories
  • adding unit tests
  • changing logic in create batch job def and prep args
    • allowing certain public images as they are (previously considered private if not valid ecr uri)

Testing

@rpmcginty rpmcginty requested review from njmei and a team June 2, 2024 22:49
@@ -15,6 +17,12 @@
)


def is_valid_docker_uri(uri):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just like we an S3URI ValidatedStr class (probably should be renamed to S3Uri), it might be useful to have an EcrUri ValidatedStr class?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We have an ECR URI but it is only for private ecr repos. It might be a little hard because we extract account and region from the registry uri. I was thinking about backporting this, but this might be a little bit difficult.

I can create a separate validated str if you want me to.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

okay I will pick up this work in a follow up

@rpmcginty rpmcginty merged commit 38049fd into main Jun 4, 2024
4 checks passed
@rpmcginty rpmcginty deleted the feature/update-demand-lambda-handlers branch June 4, 2024 19:44
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