[component] Bump maximum length for component names to 1024 characters

[mx-psi]

Description

Bumps component name length limit to 1024 characters.

Link to tracking issue

Fixes #10816

Testing

Added some unit tests.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.65%. Comparing base (eb6bc6a) to head (93dc78e).

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10818      +/-   ##
==========================================
+ Coverage   91.64%   91.65%   +0.01%     
==========================================
  Files         406      406              
  Lines       18985    18987       +2     
==========================================
+ Hits        17398    17402       +4     
+ Misses       1227     1226       -1     
+ Partials      360      359       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[TylerHelmuth]

Since this is happening during startup, should the length check be broken our from the character check to improve the error message?

[djaglowski]

I appreciate the quick action on this but I'm not sure this is meaningfully better.

As far as I am aware, 63 was suggested as the limit based on k8s. Is there a specific reason why we would settle on that or 127?

[mx-psi]

Is there a specific reason why we would settle on that or 127?

Having some limit is useful so that we can know under what contexts we can use a component.ID (since there are systems that impose some length limit). Raising it to 127 means we drop compatibility with k8s, but compatibility with existing users seems more important.

I don't have a good justification for 127 other than keeping this number as small as possible makes our life easier in the future. We can raise the limit further if we get more user reports of things breaking with that limit, but lowering the limit will be much harder after 1.0 so I would like to keep a reasonable limit.

I don't think we (or at least, I) have a good criteria as to what the limit should be, but I think there is an argument for having some limit, and this seems like something that would work for the users that have reported issues with this so far.

[djaglowski]

this seems like something that would work for the users that have reported issues with this so far

As a representative of the vendor affected by #10816, I don't believe this is accurate. The autogenerated names are unbounded because there was previously no limit. Any limit is a technically a breaking change, though in practice a large limit should not be a problem.

Can we be less restrictive for now, say 1024, and consider a stricter limit prior to 1.0 if someone articulates a use case?

[mwear]

The main goal of this change was to prevent whitespace in the name. We didn't have a length requirement previously, it crept in by looking at what we were doing for the type. We can remove it entirely and add it later when and if we have a need for it.

We can also set it sufficiently high, if we can agree on what that number should be. 1024 would be a reasonable candidate that is both high, but not unlimited in length.

[andykellr]

I would suggest removing the limit until it produces a clear error message indicating that the length is the problem.

Once there is a clear error message, a limit could be added. I would support something higher than 128. 1024 sounds reasonable.

[mx-psi]

@djaglowski

As a representative of the vendor affected by #10816, I don't believe this is accurate. The autogenerated names are unbounded because there was previously no limit. Any limit is a technically a breaking change, though in practice a large limit should not be a problem.

Ah, I hadn't understood that the length was unbouded in your use case, sorry.

Can we be less restrictive for now, say 1024

Done!

and consider a stricter limit prior to 1.0 if someone articulates a use case?

This is okay to me. It's likely that if we get a use case it will be far in the future when this is much harder to change, but this sounds like the best we can do for now.

---

@mwear

We can remove it entirely and add it later when and if we have a need for it.

After 1.0, we are likely to have a much harder time reducing this limit (this is a breaking change after all), so I prefer to start with a large limit if that is acceptable to everyone.

---

@andykellr

I would suggest removing the limit until it produces a clear error message indicating that the length is the problem.

I separated the two error messages. I think this should solve your concern.

[BinaryFissionGames]

Thanks for jumping on this!

[djaglowski]

Thanks @mx-psi!