Skip to content

Make imports idempotent #34

Merged
rranjan3 merged 12 commits intomainfrom
rranjan3/bit-updates
Apr 15, 2025
Merged

Make imports idempotent #34
rranjan3 merged 12 commits intomainfrom
rranjan3/bit-updates

Conversation

@rranjan3
Copy link
Copy Markdown
Contributor

@rranjan3 rranjan3 commented Apr 10, 2025

PULL DESCRIPTION

Make imports idempotent. Since the registration is a multi-stage action

  • allow an import in second iteration if the failure happened midway in the 1st
  • report duplicate only if the host registration as well as instance creation is already done
  • no deep compare to allow update of fields, if instance already created

Impact Analysis

Info Please fill out this column
Root Cause Specifically for bugs, empty in case of no variants
Jira ticket ITEP-25036

CODE MAINTAINABILITY

  • Added required new tests relevant to the changes and the URL has been included
  • Updated Documentation as relevant to the changes
  • PR change contains code related to security
  • PR introduces changes that break compatibility with other modules/services (If YES, please provide description)

Code must act as a teacher for future developers

Signed-off-by: Rajeev Ranjan <rajeev2.ranjan@intel.com>
Signed-off-by: Rajeev Ranjan <rajeev2.ranjan@intel.com>
Signed-off-by: Rajeev Ranjan <rajeev2.ranjan@intel.com>
@rranjan3
Copy link
Copy Markdown
Contributor Author

rranjan3 commented Apr 10, 2025

image

1.) Faulty os profile is deliberately induced
2.) Import attempted which fails with no data in EIM persisted(Pre-validation ensures, even host registration is not done if invalid input)
3.) profile corrected & import reattempted
4.) Import succeeds by registering host & then creating instance.

@rranjan3 rranjan3 marked this pull request as ready for review April 10, 2025 15:09
@rranjan3 rranjan3 requested a review from pierventre April 10, 2025 15:09
Comment thread bulk-import-tools/internal/errors/errors.go
@pierventre
Copy link
Copy Markdown
Contributor

image

1.) Faulty os profile is deliberately induced 2.) Import attempted which fails with only host registration 3.) profile corrected & import reattempted 4.) Import succeeds by creating instance.

do we have unit tests emulating a similar scenario?

@rranjan3
Copy link
Copy Markdown
Contributor Author

image
1.) Faulty os profile is deliberately induced 2.) Import attempted which fails with only host registration 3.) profile corrected & import reattempted 4.) Import succeeds by creating instance.

do we have unit tests emulating a similar scenario?

Need to add e2e test.

@pierventre
Copy link
Copy Markdown
Contributor

lgtm

Signed-off-by: Rajeev Ranjan <rajeev2.ranjan@intel.com>
Signed-off-by: Rajeev Ranjan <rajeev2.ranjan@intel.com>
Signed-off-by: Rajeev Ranjan <rajeev2.ranjan@intel.com>
Signed-off-by: Rajeev Ranjan <rajeev2.ranjan@intel.com>
Comment thread bulk-import-tools/README.md
Copy link
Copy Markdown
Contributor

@pierventre pierventre left a comment

Choose a reason for hiding this comment

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

lgtm - make sure it is visible in the README the overriding behavior between global vs user input

@rranjan3 rranjan3 merged commit 570f1ce into main Apr 15, 2025
13 checks passed
@rranjan3 rranjan3 deleted the rranjan3/bit-updates branch April 15, 2025 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants