-
Notifications
You must be signed in to change notification settings - Fork 38
WIP: Fix message too long #163
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.
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
base: master
Are you sure you want to change the base?
Conversation
@olivergondza , what do you think is the ultimate solution here? The trigger should check the size of CI_MESSAGE and not schedule the job if it is too big? And it should just print out a warning? |
Given the symptoms are quite surprising to investigate, I want to prevent such message to ever get into environment variable. I see 2 ways to go:
|
Another option would be to truncate the variable value and mark the build as unstable. I'm not sure if the second part is even possible or not.
I'm not sure this is possible. I believe the environment variable is always set. I also think in many cases users have no control over the message itself, it's coming from some publisher that's a black box to them. I think it may also be the case that many users don't even care about the variable. We could also potentially do both. Default to failing the job, but provide an option to change that to unstable and submit with a truncated message. |
Truncate + unstable will avoid the problem, but I would still go with failing the build. Unstable primarily means "we build it, but there are test failures, warnings, compliance check problems, etc", and that it might not attract the desired attention especially if the build was unstable before. Not to mention that the build will possibly fail later, trying to parse the message or use its incomplete form propagating the error silently further down the chain and making the cause harder to discover. I like to fail fast and loud.
Can be, that is why I believe they need a way to say "I know my message can be too long for an envvar, so put it in the file instead". |
@olivergondza, how exactly would this work? When you add a
Yeah, and I've already decided the next release won't be backward-compatible, so this may be the time. |
When message grows too long (+128kb), it can cause build cannot launch OS processes because of that.
(Filing early to see the reproducer is working in public CI)