Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Catch all exceptions during migration #1893

Merged
merged 5 commits into from
Sep 7, 2023
Merged

Catch all exceptions during migration #1893

merged 5 commits into from
Sep 7, 2023

Conversation

jsangmeister
Copy link
Contributor

If exceptions other than MigrationException happen during a migration, they are currently not logged anywhere. They should instead be logged.

@jsangmeister jsangmeister added the enhancement General enhancement which is neither bug nor feature label Sep 5, 2023
@jsangmeister jsangmeister added this to the 4.1 milestone Sep 5, 2023
@jsangmeister jsangmeister mentioned this pull request Sep 5, 2023
Closed
@r-peschke r-peschke assigned jsangmeister and unassigned r-peschke Sep 6, 2023
@jsangmeister
Copy link
Contributor Author

@r-peschke did you mean to approve this?

r-peschke
r-peschke approved these changes Sep 7, 2023
View reviewed changes
Copy link
Member

@r-peschke r-peschke left a comment

Choose a reason for hiding this comment

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

What's about the yml changes. Please write an issue if you change there.

For the migration part: I miss the reason why you do this and/or a test.

@jsangmeister
Copy link
Contributor Author

What's about the yml changes. Please write an issue if you change there.

It's a simple version upgrade, I don't think a separate issue is needed here. If you wish, I could extract it into its own PR, but I think that's overkill. We should think about adding a dependabot.yml file analogous to e.g. the autoupdate service: https://github.com/OpenSlides/openslides-autoupdate-service/blob/main/.github/dependabot.yml

For the migration part: I miss the reason why you do this and/or a test.

This was not tested before and is really hard to test, so I didn't add a test. The reason why I did this is linked earlier in this thread.

@jsangmeister jsangmeister enabled auto-merge (squash) September 7, 2023 14:29
@jsangmeister jsangmeister merged commit 5605f2b into OpenSlides:main Sep 7, 2023
4 checks passed
peb-adr added a commit that referenced this pull request Sep 15, 2023
@peb-adr
Merge main into stable/4.0.x. Update 20230915
a3d2cd1 
* commit '343326a220f149fb06e91d96df85bfb339b59113':
  Use get_all instead of filter for migration 46 (#1910)
  Fixed readding point of order speaker (#1909)
  I1906 migrations 0045, 0046 replace get-all by filter (#1908)
  Support point of order speakers in re-add-speaker (#1894) (#1905)
  Remove all topic.tag_ids from database (#1901)
  update all new fields (#1900)
  Catch all exceptions during migration (#1893)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@r-peschke r-peschke r-peschke approved these changes

Assignees

@r-peschke r-peschke

Labels
enhancement General enhancement which is neither bug nor feature
Projects
None yet
Milestone
4.1
Development

Successfully merging this pull request may close these issues.
2 participants
@jsangmeister @r-peschke