Differential D458

don't try to create >255 chars long artifact filenames
ClosedPublic
Actions

Authored by kparal on Jul 20 2015, 11:12 AM.

Details

Reviewers
lbrabec
mkrizek
jskladan
Maniphest Tasks
T530: Depcheck fails to create per-update log
Commits
rDEPCKf47dab8eb46d: don't try to create >255 chars long artifact filenames
Summary

This happened for large Bodhi updates with many builds before it got
assigned an update id.

Test Plan

tested manually

Diff Detail

Repository
rDEPCK task-depcheck
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kparal retitled this revision from to don't try to create >255 chars long artifact filenames.Jul 20 2015, 11:12 AM
kparal updated this object.
kparal edited the test plan for this revision. (Show Details)
kparal added reviewers: jskladan, mkrizek, lbrabec.
kparal added a task: T530: Depcheck fails to create per-update log.
This revision is now accepted and ready to land.Jul 20 2015, 11:26 AM
jskladan added a comment.Jul 20 2015, 11:29 AM

you nasty ninja!

jskladan requested changes to this revision.Jul 20 2015, 11:36 AM
jskladan added inline comments.
depcheck/squash_results.py
176–177

item = item[:255-len('....%s.log' % result.keyvals['arch']) + '...'

This IMHO has the same output, but is more readable, how about that?

This revision now requires changes to proceed.Jul 20 2015, 11:36 AM
kparal added inline comments.Jul 20 2015, 11:43 AM
depcheck/squash_results.py
176–177

Another option would be this:

exceed_len = len(filename) - 255
item = item[:-exceed_len-len('...')] + '...'

I should have done it like that in the first place. I like that I don't need to repeat the structure with all the arch and log suffix etc (which can change).

But if you prefer the way you proposed it more, why not. I'd probably do arch = result.keyvals['arch'] first, to make it more readable.

jskladan accepted this revision.Jul 20 2015, 11:54 AM

Accepted with the changes mentioned in the comments.

depcheck/squash_results.py
176–177

Another option would be this: ...

This revision is now accepted and ready to land.Jul 20 2015, 11:54 AM
Closed by commit rDEPCKf47dab8eb46d: don't try to create >255 chars long artifact filenames (authored by kparal). · Explain WhyJul 20 2015, 11:59 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathPackages
Mdepcheck/squash_results.py (14 lines)

Diff 1210

View Options

depcheck/squash_results.py

Show First 20 Lines • Show All 163 Lines • ▼ Show 20 Line(s)163 try:
164 storedir = os.path.abspath(artifactsdir) 164​ storedir = os.path.abspath(artifactsdir)
165 log.logger.debug('Writing individual logs into: %s', storedir) 165​ log.logger.debug('Writing individual logs into: %s', storedir)
166 166​
167 if not os.path.exists(storedir): 167​ if not os.path.exists(storedir):
168 os.makedirs(storedir) 168​ os.makedirs(storedir)
169 169​
170 # save logs 170​ # save logs
171 for result in results: 171​ for result in results:
172 filename = "%s.%s.log" % (os.path.join(storedir, result.item), result.keyvals['arch']) 172​ item = result.item
173 fout = open(filename, 'w') 173​ filename = "%s.%s.log" % (item, result.keyvals['arch'])
174​ # Bodhi update titles can be very long, we need to trim them
175​ if len(filename) > 255: # the max file name length for most Linux filesystems
176​ exceed_len = len(filename) - 255
177​ item = item[:-exceed_len-len('...')] + '...'
jskladanUnsubmitted
Not Done

item = item[:255-len('....%s.log' % result.keyvals['arch']) + '...'

This IMHO has the same output, but is more readable, how about that?

kparalAuthorUnsubmitted
Not Done

Another option would be this:

exceed_len = len(filename) - 255
item = item[:-exceed_len-len('...')] + '...'

I should have done it like that in the first place. I like that I don't need to repeat the structure with all the arch and log suffix etc (which can change).

But if you prefer the way you proposed it more, why not. I'd probably do arch = result.keyvals['arch'] first, to make it more readable.

jskladanUnsubmitted
Not Done

Another option would be this: ...

178​ filename = "%s.%s.log" % (item, result.keyvals['arch'])
179​ assert len(filename) <= 255
180​ filepath = os.path.join(storedir, filename)
181​ fout = open(filepath, 'w')
174 fout.write(check.export_TAP([result], checkname='depcheck')) 182​ fout.write(check.export_TAP([result], checkname='depcheck'))
175 fout.close() 183​ fout.close()
176 result.artifact = filename 184​ result.artifact = filepath
177 185​
178 except OSError as e: 186​ except OSError as e:
179 log.logger.error('Individual logs not saved into %s, an error occured. ' 187​ log.logger.error('Individual logs not saved into %s, an error occured. '
180 'Raising.', artifactsdir) 188​ 'Raising.', artifactsdir)
181 raise e 189​ raise e
182 190​