- 
                Notifications
    
You must be signed in to change notification settings  - Fork 64
 
cockpit-image-builder: export blueprints (RHEL-123840) #3779
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.
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
base: main
Are you sure you want to change the base?
Conversation
cc28ed9    to
    6ab29ba      
    Compare
  
    
          Codecov Report❌ Patch coverage is  
 @@            Coverage Diff             @@
##             main    #3779      +/-   ##
==========================================
- Coverage   81.00%   80.95%   -0.05%     
==========================================
  Files         217      217              
  Lines       25079    25102      +23     
  Branches     2494     2496       +2     
==========================================
+ Hits        20315    20322       +7     
- Misses       4736     4752      +16     
  Partials       28       28              
 Continue to review full report in Codecov by Sentry. 
 🚀 New features to boost your workflow:
  | 
    
06c08e2    to
    92d0be5      
    Compare
  
    92d0be5    to
    c94e4b3      
    Compare
  
    16c1ba9    to
    fc8fbb2      
    Compare
  
    Export the blueprint as TOML on premise, and not as JSON. The expectation is that users might want to use this blueprint with composer-cli or image-builder-cli. Because the behaviour is different between the cockpit and hosted UIs, refrain from making the backend api abstraction.
fc8fbb2    to
    fc64d5a      
    Compare
  
    Also verify the contents of the exported blueprint. For hosted this is not important as there the exported blueprint is just forwarded from image-builder-crc. But in cockpit-image-builder all blueprint management is handled inside of the plugin itself, thus it is important to verify the contents of the export.
fc64d5a    to
    b1f1940      
    Compare
  
    | 
           /retest  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@croissanne it looks great, I left 2 small suggestion, other then that it looks good to me
| import { EllipsisVIcon } from '@patternfly/react-icons'; | ||
| import TOML from 'smol-toml'; | ||
| 
               | 
          ||
| // The exported contents are different between hosted and on prem, so | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you think maybe write it little different?
// The hosted UI exports JSON, while the Cockpit plugin exports TOML.
// Because the blueprint formats are not aligned, relying on a generic 'backendApi'
// would be misleading. Import and handle each environment explicitly.
| // and on prem, so just import both separately. | ||
| import { selectSelectedBlueprintId } from '../../store/BlueprintSlice'; | ||
| import { useLazyExportBlueprintCockpitQuery } from '../../store/cockpit/cockpitApi'; | ||
| import type { Blueprint as CloudApiBlueprint } from '../../store/cockpit/composerCloudApi'; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is litte confusing to use CloudApi and also cockpit -
maybe 'cockpitExportResponse' instead of  CloudApiBlueprint'
Uh oh!
There was an error while loading. Please reload this page.