Skip to content

Conversation

@rd-pong
Copy link
Contributor

@rd-pong rd-pong commented Apr 13, 2023

  • Create shallow canary marker for eu-south-2, ap-southeast-3, me-central-1, eu-central-2, since some registries are not available in those regions.
  • Add training and hosting to shallow canary
  • Add placeholder values in replacement_values.py to avoid KeyError (not found)

Issue #, if available:

Description of changes:

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

Add placeholder values in replacement_values.py to avoid KeyError
@ack-prow ack-prow bot requested a review from jljaco April 13, 2023 17:03
@ack-prow ack-prow bot added the approved label Apr 13, 2023
@rd-pong rd-pong requested review from ananth102 and removed request for jljaco and surajkota April 13, 2023 17:03


@service_marker
@pytest.mark.shallow_canary
Copy link
Member

Choose a reason for hiding this comment

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

This marker means if we run pytest with this marker then run this test. In your shell-script you are still running these tests with the regions in this ^(eu-south-2|ap-southeast-3|me-central-1|eu-central-2) Correct if I'm wrong but isn't the goal of this pr to not run these tests in those regions since image is missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We want to run shallow canary, which includes training and hosting, in eu-south-2, ap-southeast-3, me-central-1, eu-central-2. Training and hosting does not require missing images such as xgboost, debugger and clarify.

@rd-pong
Copy link
Contributor Author

rd-pong commented Apr 13, 2023

Verified shallow canary if-else logic (run_test.sh) in CodeBuild. If service region is one of the four (eu-south-2, ap-southeast-3, me-central-1, eu-central-2), then run test_endpoint, test_endpoint_config and test_trainingjob.

Copy link
Member

@ryansteakley ryansteakley left a comment

Choose a reason for hiding this comment

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

/lgtm

@ack-prow ack-prow bot added the lgtm Indicates that a PR is ready to be merged. label Apr 13, 2023
@ack-prow
Copy link

ack-prow bot commented Apr 13, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rd-pong, ryansteakley

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [rd-pong,ryansteakley]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ack-prow ack-prow bot merged commit 724b24c into aws-controllers-k8s:main Apr 13, 2023
rd-pong added a commit that referenced this pull request Apr 13, 2023
- Create shallow canary marker for eu-south-2, ap-southeast-3, me-central-1, eu-central-2, since some registries are not available in those regions.
- Add training and hosting to shallow canary
- Add placeholder values in replacement_values.py to avoid KeyError (not found)

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
rd-pong added a commit to rd-pong/sagemaker-controller that referenced this pull request Apr 18, 2023
- Create shallow canary marker for eu-south-2, ap-southeast-3, me-central-1, eu-central-2, since some registries are not available in those regions.
- Add training and hosting to shallow canary
- Add placeholder values in replacement_values.py to avoid KeyError (not found)

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
@rd-pong rd-pong deleted the shallow-canary branch April 19, 2023 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants