Summary
The user is discussing an issue with Flyte regarding the removal of trailing newlines in secrets by Flytekit, which aligns with Flyteadmin's behavior. They are questioning the necessity of changing this legacy behavior, suggesting it would require a feature flag and deprecation notices. The user is looking for quick solutions, such as retrying, and is assessing whether the issue is significant enough to address. They express a preference for closing the related issue as "Won't fix."
chenvincent610
I see. I will close my pr as well. Thanks for the clarification~
curupa
Got it. That's what I was thinking. In that case, I'm going to close that issue as Won't fix since the risks do not outweigh the benefits.
habuelfutuh
The thinking was that somebody may have taken a dependency on that behavior (as we did in Lyft) and it will break/surprise them if we remove it...
curupa
I don't think there's a great way around that.
The original https://github.com/flyteorg/flyteadmin/pull/168/|PR doesn't mention why this legacy behavior was kept (do you remember, <@UNW4VP36V>?).
In fact, re-reading the issue I'd be in favor of simply closing it as Won't fix.
habuelfutuh
it's a legacy behavior that we maintained unfortunately... the only way around it will require a feature flag and deprecation notices... etc.
<@USU6W5ATA> <@U0265RTUJ5B> do you think there is a quick way around that? potentially retrying? Is this issue important to address?
chenvincent610
Hi, all. I am currently working on this https://github.com/flyteorg/flyte/issues/2559|issue. After a brief discussion with Kevin Su, we found that <https://github.com/flyteorg/flytekit/blob/1b67f16ce768225f034fe4698a43e30047817fab/flytekit/core/context_manager.py#L383|Flytekit handles trailing newlines> in secrets by removing them, which is consistent with how Flyteadmin handles it in the current implementation. I was wondering if there are any reasons, aside from consistency mentioned in the issue, that we might want to change this behavior. Thanks!