Skip to content

fix: Create a new File when rewriting artifact index#26655

Merged
jjbayer merged 7 commits into
masterfrom
fix/immutable-files
Jun 16, 2021
Merged

fix: Create a new File when rewriting artifact index#26655
jjbayer merged 7 commits into
masterfrom
fix/immutable-files

Conversation

@jjbayer

@jjbayer jjbayer commented Jun 16, 2021

Copy link
Copy Markdown
Member

Consider File immutable, so create a new one any time the artifact index is updated.

Comment thread src/sentry/models/releasefile.py Outdated
if create:
self._ensure_releasefile()

# No race condition here, because artifact index is never deleted

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless user deletes all release artifacts via sourcemaps endpoint.

@jjbayer jjbayer marked this pull request as ready for review June 16, 2021 14:52
@jjbayer jjbayer requested review from jan-auer and untitaker June 16, 2021 14:53
releasefile = None

if releasefile is None:
index_data = None

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only branch where index_data can be None, and then the only remaining thing this function does is yield None. Can we bail out here instead?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could probably do something like

yield None
return

but I'm not sure if that would improve readability.

@jjbayer jjbayer merged commit 0c3905b into master Jun 16, 2021
@jjbayer jjbayer deleted the fix/immutable-files branch June 16, 2021 17:06
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants