Improve execution name readability

[wayner0628]

Tracking issue

Why are the changes needed?

• Improve execution name readability

What changes were proposed in this pull request?

• Add EnableHumanHash config in flyteadmin/pkg/runtime/interfaces/application_configuration.go
• Add human hash feature in flyteadmin/pkg/common/executions.go

How was this patch tested?

unit testing and integration testing

Setup process

To enable human-readable execution names, add the following configuration:

flyteadmin:
  featureGates:
    enableHumanHash: true

Screenshots

Check all the applicable boxes

• [x] I updated the documentation accordingly.
• [x] All new and existing tests passed.
• [x] All commits are signed-off.

Related PRs

https://github.com/flyteorg/flytekit/pull/2678

Docs link

[welcome[bot]]

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).

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 36.17%. Comparing base (5c147ef) to head (f2b7b7f). Report is 157 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5637      +/-   ##
==========================================
- Coverage   36.17%   36.17%   -0.01%     
==========================================
  Files        1302     1303       +1     
  Lines      109614   109611       -3     
==========================================
- Hits        39653    39649       -4     
- Misses      65816    65819       +3     
+ Partials     4145     4143       -2     

Flag	Coverage Δ
unittests-datacatalog	51.37% <ø> (ø)
unittests-flyteadmin	55.28% <100.00%> (-0.02%)	:arrow_down:
unittests-flytecopilot	12.17% <ø> (ø)
unittests-flytectl	62.18% <ø> (ø)
unittests-flyteidl	7.12% <ø> (ø)
unittests-flyteplugins	53.34% <ø> (ø)
unittests-flytepropeller	41.71% <ø> (ø)
unittests-flytestdlib	55.35% <ø> (+0.02%)	:arrow_up:

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[wayner0628]

Hello @pingsutw @eapolinario,

I've made the following changes based on your feedback:

1. Removed all unnecessary error handlers.
2. Added a comment explaining why errors in humanhash generation can be safely ignored.
3. Updated the humanhash package version to v1.1.0.
4. Renamed the configuration setting to EnableFriendlyNames.
5. Extend the name to 4 subwords.
6. Now reading the config directly within the getExecutionName function, and adjusted the file path accordingly.
7. Updated the unit tests to use mocking objects, as the config is now read within the function.

Please review these changes and let me know if you have any suggestions or concerns.

Thank you!

[wayner0628]

Hi @pingsutw @eapolinario,

I've thoroughly investigated the issue but couldn't find any connection between my changes and the CI unit test failure. Additionally, there are no error messages logged that could provide further insight. Could you please help retrigger the CI build? Thank you!

[wayner0628]

I just reverted the flytectl changes. It seems like we can't change that in the same PR. You could create a follow-up PR to update flytectl.

I noticed some issues still persist, so I'll continue checking them.

[welcome[bot]]

Congrats on merging your first pull request! 🎉

[ddl-ebrown]

FYI - we implemented something similar in our custom auth gateway, as we weren't sure if this would be something Flyte would use.

A few differences with our implementation:

• We use https://github.com/dustinkirkland/golang-petname for the names
• To create enough uniqueness, we use 3 terms PLUS a 4 character suffix (we wanted execution names to be unique across all of Flyte, not just within a specific project / domain). The suffix is produced using https://github.com/matoous/go-nanoid
  • Humanhash looks to be using a similar or the same word list as golang-petname. 3 words in golang-petname is about 53 million combinations, which we didn't think was enough uniqueness. With the 4 character suffix, we end up with about 89 trillion combinations.

We would consider removing our implementation in favor of upstream if we can get a better uniqueness guarantee. Would you be willing to modify the friendly naming here to support that?

We get names like hardly-maximum-sculpin-rax1, commonly-maximum-narwhal-jubn, officially-pure-alien-tm9a , etc

[pingsutw]

@ddl-ebrown, we could make suffix opt-in. Need to add one more field to the config map. something like

flyteadmin
  featureGates:
    enableFriendlyNames: true
  execution_name_with_random_suffix: true

Feel free to open a PR!

[kumare3]

This is pretty cool - cc @katrogan