tanishqgandhi1908 commented on code in PR #8546:
URL: https://github.com/apache/texera/pull/8546#discussion_r4031649212


##########
computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/CuratedImageResourceSpec.scala:
##########
@@ -301,12 +299,10 @@ class CuratedImageResourceSpec extends AnyFlatSpec with 
Matchers {
       "sha256:1111111111111111111111111111111111111111111111111111111111111111"
   }
 
-  "the feature flag" should "be off unless a deployment turns it on" in {
-    CuratedImageConfig.enabled shouldBe false
-  }
-
-  it should "start no unit from a curated image while it is off" in {
-    CuratedImageResource.readyImageFor(1) shouldBe None
+  // On by default now that the pages to manage and choose images have 
shipped. A
+  // deployment short of node disk, or unwilling to offer them, sets it back 
to false.
+  "the feature flag" should "be on unless a deployment turns it off" in {

Review Comment:
   Agreed — removing that left the guard untested, which is the real loss, not 
the flag value.
   
   I couldn't use the env hook though: `CuratedImageConfig.enabled` is a `val` 
read once at class load, so a test can't turn the feature off around a call. 
That's what made the original test impossible to keep once the flag shipped on.
   
   Instead the rule now lives in `startableRef(enabled, status, sourceRef, 
sourceDigest)`, which `readyImageFor` delegates to. Three cases are stated 
directly — off, not yet checked, ready — with no env and no database.
   
   Checked it actually guards the thing you were worried about: deleting the 
`if (!enabled) None` branch now fails the suite.



##########
frontend/src/app/common/component/computing-unit-create-modal/computing-unit-create-modal.component.ts:
##########
@@ -171,15 +182,36 @@ export class ComputingUnitCreateModalComponent implements 
OnInit, OnChanges {
   ngOnChanges(changes: SimpleChanges): void {
     if (changes["visible"]?.currentValue === true) {
       this.resetAdvancedSettings();
+      this.loadCuratedImages();
     }
   }
 
+  /**
+   * Read when the dialog opens rather than in ngOnInit: both hosts render 
this component
+   * unconditionally, so ngOnInit runs once at page load. An image that became 
ready since
+   * then would never appear, and one failed read would hide the field for the 
session.
+   *
+   * Readable by any signed-in user. A deployment with curated images off 
answers 503, and
+   * a user who never sees the dropdown gets exactly today's behaviour.
+   */
+  private loadCuratedImages(): void {
+    this.cuImageService
+      .list()
+      .pipe(untilDestroyed(this))
+      .subscribe({
+        next: images => (this.curatedImages = images.filter(isStartable)),
+        error: () => (this.curatedImages = []),

Review Comment:
   Fixed — and I don't think it's only a nit, since the symptom is "the 
dropdown disappeared" with nothing to go on.
   
   Only 503 is silent now. The field still empties either way, so the unit 
falls back to the deployment's image, but anything else is reported. Two tests: 
a 500 is surfaced, a 503 is not.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to