Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Increase allowedExecutionNameLength to 63 #466

Merged
merged 2 commits into from
Aug 31, 2022

Conversation

rahul-theorem
Copy link
Contributor

@rahul-theorem rahul-theorem commented Aug 30, 2022

Signed-off-by: Rahul Mehta [email protected]

TL;DR

Increase allowedExecutionNameLength from 20 to 63 to allow more legible human-readable execution IDs

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

See title/summary

Tracking Issue

Closes flyteorg/flyte#2824

Follow-up issue

NA

@welcome
Copy link

welcome bot commented Aug 30, 2022

Thank you for opening this pull request! 🙌

These tips will help get your PR across the finish line:

  • Most of the repos have a PR template; if not, fill it out to the best of your knowledge.
  • Sign off your commits (Reference: DCO Guide).

@rahul-theorem
Copy link
Contributor Author

Just fixed the failing test, @katrogan would it be possible to re-run the CI checks (looks like it's required since this is my first PR to flyteadmin)?

@codecov
Copy link

codecov bot commented Aug 31, 2022

Codecov Report

Merging #466 (658611f) into master (3ae3bc6) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #466   +/-   ##
=======================================
  Coverage   61.64%   61.64%           
=======================================
  Files         157      157           
  Lines       11305    11305           
=======================================
  Hits         6969     6969           
  Misses       3613     3613           
  Partials      723      723           
Flag Coverage Δ
unittests 60.57% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/manager/impl/validation/execution_validator.go 73.80% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@katrogan katrogan merged commit b962ed7 into flyteorg:master Aug 31, 2022
@welcome
Copy link

welcome bot commented Aug 31, 2022

Congrats on merging your first pull request! 🎉

@rahul-theorem rahul-theorem deleted the patch-1 branch September 6, 2022 00:50
eapolinario pushed a commit that referenced this pull request Sep 6, 2023
* Increase `allowedExecutionNameLength` to 63

Signed-off-by: Rahul Mehta <[email protected]>

* Update TestValidExecutionIdInvalidLength

Signed-off-by: Rahul Mehta <[email protected]>

Signed-off-by: Rahul Mehta <[email protected]>
Signed-off-by: Rahul Mehta <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[REQUEST] Increase execution ID to 63 characters
2 participants