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.

Sign up for GitHub

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

Jump to bottom

Sagemaker Operator Character limit fix #45551

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Sagemaker Operator Character limit fix #45551

wants to merge 2 commits into from
+7 −1

Conversation

dirkrkotzeml
Copy link

@dirkrkotzeml dirkrkotzeml commented Jan 10, 2025 •
edited by eladkal
Loading

… SageMaker 64 character length constraint

SageMakerProcessingJobs have a hard limit of 64 characters for the ProcessingJobName.
In the SageMakerBaseOperator there is a check for uniqueness for the name.
In the case that a name is not unique it adds a timestamp to prevent a potential collision, however there is no check to prevent the updated - from exceeding 64 characters. This causes the creation of the SageMakerProcessingJob to fail.

In the SageMaker Pipelines SDK they truncate the base name before adding the timestamp, therefor we took a similar approach for consistency purposes.

Closes: #45550

@boring-cyborg boring-cyborg bot added area:providers provider:amazon-aws AWS/Amazon - related issues labels Jan 10, 2025
@boring-cyborg boring-cyborg
Copy link

boring-cyborg bot commented Jan 10, 2025

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our pre-commits will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
  • Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
  • Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: [email protected]
    Slack: https://s.apache.org/airflow-slack

@eladkal eladkal requested review from ferruzzi and vincbeck January 11, 2025 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eladkal eladkal Awaiting requested review from eladkal eladkal is a code owner

@o-nikolas o-nikolas Awaiting requested review from o-nikolas o-nikolas is a code owner

@ferruzzi ferruzzi Awaiting requested review from ferruzzi

@vincbeck vincbeck Awaiting requested review from vincbeck

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
area:providers provider:amazon-aws AWS/Amazon - related issues
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

SageMakerBaseOperator character limit bug
2 participants
@dirkrkotzeml @dirkkotze-work