Skip to content

Conversation

@nhlien93
Copy link
Contributor

@nhlien93 nhlien93 commented Sep 9, 2020

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build was run locally and any changes were pushed
  • Lint has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Cannot define snapshot parameters. Staged plugins do have a "snapshot parameter" that is defined to have a resync boolean.

What is the new behavior?

  • snapshot parameters definition is required in the schema file now.
  • src code for pre_snapshot / post_snapshot in linked object all have optional_snapshot_parameters now (direct and staged)

Does this introduce a breaking change?

  • Yes
  • No

This change will mean anyone that wants to build a plugin with this version of the SDK will have to update two things:
the schema file needs to have the snapshot_parameters_definition and the source code needs to add the optional_snapshot_parameters to the pre_snapshot / post_snapshot operations.

Tests were run on the appgate side. Also added new unit test to verify that dvp init -> dvp build works because init should always create a buildable plugin.

@nhlien93 nhlien93 force-pushed the snapshotparams branch 3 times, most recently from d6f968e to cdda079 Compare September 17, 2020 20:15
@nhlien93 nhlien93 marked this pull request as ready for review September 25, 2020 17:21
direct_source=direct_source,
repository=repository,
source_config=source_config,
optional_snapshot_parameters=snapshot_parameters)
Copy link
Contributor

Choose a reason for hiding this comment

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

I love the idea of putting the word "optional" right in the argument name here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

haha it was your idea! so thank you 🥇

Copy link
Contributor

Choose a reason for hiding this comment

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

It was?? I have no memory of this.

@nhlien93 nhlien93 merged commit 2dfbee8 into delphix:develop Sep 25, 2020
@nhlien93 nhlien93 linked an issue Sep 28, 2020 that may be closed by this pull request
@nhlien93 nhlien93 deleted the snapshotparams branch September 28, 2020 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Update SDK to add SnapshotParametersDefinition in uploaded artifact

4 participants