Skip to content
This repository has been archived by the owner on Nov 14, 2024. It is now read-only.

BackupService backwards compatibility #6040

Merged
merged 9 commits into from
May 26, 2022
Merged

BackupService backwards compatibility #6040

merged 9 commits into from
May 26, 2022

Conversation

jeremyk-91
Copy link
Contributor

Goals (and why):

==COMMIT_MSG==
==COMMIT_MSG==

Implementation Description (bullets):

Testing (What was existing testing like? What have you done to improve it?):

Concerns (what feedback would you like?):

Where should we start reviewing?:

Priority (whenever / two weeks / yesterday):

@jeremyk-91 jeremyk-91 requested a review from Jolyon-S May 18, 2022 11:53
@changelog-app
Copy link

changelog-app bot commented May 18, 2022

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description
Backup Service can now deserialise metadata persisted from older formats.

Check the box to generate changelog(s)

  • Generate changelog entry

Copy link
Contributor

@Jolyon-S Jolyon-S left a comment

Choose a reason for hiding this comment

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

gtg

log.info(
"Successfully loaded file",
SafeArg.of("fileName", file.getName()),
SafeArg.of("namespace", atlasService));
Copy link
Contributor

Choose a reason for hiding this comment

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

non-blocking: you've reverted this name

When we need this object mapper, we will probably need it many times. So let's create it at most once.
@gsheasby gsheasby changed the title Jkong/269794 BackupService backwards compatibility May 25, 2022
@Jolyon-S Jolyon-S self-requested a review May 26, 2022 09:24
Copy link
Contributor

@Jolyon-S Jolyon-S left a comment

Choose a reason for hiding this comment

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

one small question, otherwise looks good

Comment on lines 55 to 56
this.legacyObjectMapper = Suppliers.memoize(
() -> OBJECT_MAPPER.copy().setPropertyNamingStrategy(PropertyNamingStrategies.KEBAB_CASE));
Copy link
Contributor

Choose a reason for hiding this comment

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

is there any reason we can't just have a second static?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we could - we most likely won't need to initialize this, but doing so can't be that expensive, and it'd make the code a little simpler.

@bulldozer-bot bulldozer-bot bot merged commit cbcb1b7 into develop May 26, 2022
@bulldozer-bot bulldozer-bot bot deleted the jkong/269794 branch May 26, 2022 13:11
@svc-autorelease
Copy link
Collaborator

Released 0.617.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants