src: Add on prem blueprints import support (HMS-4683) #2519

avitova · 2024-10-15T15:39:45Z

This pull request relates to #4683.
We want to support import on prem blueprints in our import feature. This enables importing toml blueprints that get automatically detected, and parsed.
A lot of fields we need to ignore, a lot of fields from our blueprints are not in the on-prem blueprint structure. This is our first attempt to allow on-prem blueprint, and it will get us more information whether users would use it or not. We also need to add a method to measure the usage of this feature. This is not here yet, and I would like to do it as a follow up, as it can raise discussions, and we do not want to block this feature on it.

We will bring a different solution, so eventually, we will get rid of this mapper. But the other solution that should be more "bulletproof" will take time, in the meantime we can use this one. :)

I know, that on-prem blueprints can also come in json, this will be quite easy to do, we will adjust the UI a bit for that, so that users would be able to import even on-prem json.

Feel free to ask any questions, and find bugs!

ezr-ondrej

What's the status here? should I test it out?

ezr-ondrej · 2024-10-16T14:01:00Z

src/Components/Blueprints/ImportBlueprintModal.tsx

+        const isToml = filename.endsWith('.toml');
+        const isJson = filename.endsWith('.json');
+        if (isToml) {
+          var toml = require('toml');


ugh... I'd rather import it globaly... (not sure why, but I think combining require and import is a no-no in JS)

avitova · 2024-11-05T08:46:25Z

src/Components/CreateImageWizard/steps/Packages/PackageRecommendations.tsx

@@ -77,7 +77,7 @@ const PackageRecommendations = () => {
          },
        });

-        if (response && response.data && response.data.packages.length > 0) {
+        if (response && response.data && response.data.packages && response.data.packages.length > 0) {


This is not related, but I was getting some errors, and I believe that we need to check for this too.

codecov · 2024-11-05T11:27:17Z

Codecov Report

Attention: Patch coverage is 88.00000% with 15 lines in your changes missing coverage. Please review.

Project coverage is 83.96%. Comparing base (7c99415) to head (4c804ac).

Files with missing lines	Patch %	Lines
...ueprints/helpers/onPremToHostedBlueprintMapper.tsx	80.00%	12 Missing ⚠️
src/Components/Blueprints/ImportBlueprintModal.tsx	94.91%	3 Missing ⚠️

@@            Coverage Diff             @@
##             main    #2519      +/-   ##
==========================================
+ Coverage   83.93%   83.96%   +0.03%     
==========================================
  Files         162      163       +1     
  Lines       17950    18034      +84     
  Branches     1796     1814      +18     
==========================================
+ Hits        15066    15143      +77     
- Misses       2862     2869       +7     
  Partials       22       22

Files with missing lines	Coverage Δ
...geWizard/steps/Packages/PackageRecommendations.tsx	`96.26% <100.00%> (+0.08%)`	⬆️
src/Components/Blueprints/ImportBlueprintModal.tsx	`90.18% <94.91%> (+4.76%)`	⬆️
...ueprints/helpers/onPremToHostedBlueprintMapper.tsx	`80.00% <80.00%> (ø)`

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c99415...4c804ac. Read the comment docs.

avitova · 2024-11-05T13:59:32Z

/retest

avitova · 2024-11-06T08:16:14Z

/retest

ezr-ondrej

Just few ideas inline :)

ezr-ondrej · 2024-11-08T12:36:46Z

src/Components/Blueprints/OnPremToHostedBlueprintMapper.tsx

@@ -0,0 +1,155 @@
+import {


Capital first letter is usually a React component, this is just a helper function, I'd name it with lower case first letter. possibly even place it in helpers/ sub folder.

ezr-ondrej · 2024-11-08T12:39:00Z

src/Components/Blueprints/ImportBlueprintModal.tsx

+            blueprintFromFile,
+            []
+          );
+          setImportedBlueprint(importBlueprintState);


Should we show some warning, that this is a best effort import?

Let me know how you like that one I came up with. We can adjust it. :)

ezr-ondrej · 2024-11-08T12:40:12Z

src/Components/Blueprints/ImportBlueprintModal.tsx

+        dispatch(
+          addNotification({
+            variant: 'warning',
+            title: 'No blueprint was build',


I know this is just a copy, but the warning is probably incorrect, we should say something like "File is not a valid blueprint"

I believe this was according to mocks, but I am happy to change it.

Mocks are not always 100% tought through, this case I feel like it could have been copy pasted from other mock.
What I'm interested in is whether my comment makes sense to you, or if I just don't get why this might be actually correct message :)

Well, I feel like the notification might make sense. We already inform in the warning at the bottom of the text field that the format is not compatible with our blueprints. So this one just made sense to me, so that users know that we did not create any blueprint just from the fact that they uploaded the file. We can discuss this with Kelsea, but I think this is a minor thing:)

Yeah, totaly minor, if you feel this is good, lets keep it :)

avitova · 2024-11-12T12:40:07Z

/retest

avitova · 2024-11-12T13:44:58Z

/retest

avitova · 2024-11-12T15:20:38Z

Okay, I addressed all the comments, thanks for the review!
/retest

ezr-ondrej

I'm good here, but merging with the current CI state might be difficult xD

FTR: I've tested this with toml and seems fine, just one nit :)

ezr-ondrej · 2024-11-14T17:03:54Z

src/Components/Blueprints/ImportBlueprintModal.tsx

  const [importedBlueprint, setImportedBlueprint] =
    React.useState<wizardState>();
  const [isInvalidFormat, setIsInvalidFormat] = React.useState(false);
  const [filename, setFilename] = React.useState('');
  const [isLoading, setIsLoading] = React.useState(false);
  const [isRejected, setIsRejected] = React.useState(false);
+  const [isOnPrem, setIsOnPrem] = React.useState(false);


You might want to reset this flag in onClear handler.

ezr-ondrej reviewed Oct 21, 2024

View reviewed changes

avitova force-pushed the on-prem-bp branch from a6dacf6 to a1fd69d Compare November 5, 2024 08:41

avitova marked this pull request as ready for review November 5, 2024 08:42

avitova force-pushed the on-prem-bp branch 2 times, most recently from 5d1300a to f02df31 Compare November 5, 2024 08:45

avitova commented Nov 5, 2024

View reviewed changes

avitova force-pushed the on-prem-bp branch 3 times, most recently from cb0b3ad to d4a5800 Compare November 5, 2024 11:19

avitova requested a review from ezr-ondrej November 5, 2024 11:29

ezr-ondrej reviewed Nov 8, 2024

View reviewed changes

ezr-ondrej changed the title ~~src: Add on prem blueprints import support~~ src: Add on prem blueprints import support (HMS-4683) Nov 11, 2024

avitova force-pushed the on-prem-bp branch from d4a5800 to f9c1982 Compare November 12, 2024 12:03

schutzbot added the 🌟 best practice label Nov 12, 2024

avitova force-pushed the on-prem-bp branch from f9c1982 to 683af5b Compare November 12, 2024 12:38

avitova added 2 commits November 14, 2024 17:40

Wizard: check before accessing undefined property

c30709e

src: Add on prem blueprints import support

4c804ac

ezr-ondrej force-pushed the on-prem-bp branch from 683af5b to 4c804ac Compare November 14, 2024 16:40

ezr-ondrej approved these changes Nov 14, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

src: Add on prem blueprints import support (HMS-4683) #2519

src: Add on prem blueprints import support (HMS-4683) #2519

avitova commented Oct 15, 2024 •

edited

Loading

ezr-ondrej left a comment

ezr-ondrej Oct 16, 2024

avitova Nov 5, 2024

codecov bot commented Nov 5, 2024 •

edited

Loading

avitova commented Nov 5, 2024

avitova commented Nov 6, 2024

ezr-ondrej left a comment

ezr-ondrej Nov 8, 2024

ezr-ondrej Nov 8, 2024

avitova Nov 12, 2024

ezr-ondrej Nov 8, 2024

avitova Nov 12, 2024

ezr-ondrej Nov 12, 2024

avitova Nov 12, 2024

ezr-ondrej Nov 14, 2024

avitova commented Nov 12, 2024

avitova commented Nov 12, 2024

avitova commented Nov 12, 2024

ezr-ondrej left a comment

ezr-ondrej Nov 14, 2024

src: Add on prem blueprints import support (HMS-4683) #2519

Are you sure you want to change the base?

src: Add on prem blueprints import support (HMS-4683) #2519

Conversation

avitova commented Oct 15, 2024 • edited Loading

ezr-ondrej left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Nov 5, 2024 • edited Loading

Codecov Report

avitova commented Nov 5, 2024

avitova commented Nov 6, 2024

ezr-ondrej left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avitova commented Nov 12, 2024

avitova commented Nov 12, 2024

avitova commented Nov 12, 2024

ezr-ondrej left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avitova commented Oct 15, 2024 •

edited

Loading

codecov bot commented Nov 5, 2024 •

edited

Loading