Ironclad / rivet

The open-source visual AI programming environment and TypeScript library
https://rivet.ironcladapp.com
MIT License
2.55k stars 226 forks source link

feat: add `binary` output port on `HttpCallNode` #392

Closed liaujianjie closed 1 month ago

liaujianjie commented 2 months ago

Resolves #386

Context

Previously, it wasn't possible to make a HTTP call (via HttpCallNode) and have the binary/blob data passed to an ImageNode. This PR introduces a new binary output port (alongside the existing body and json output ports) on HttpCallNode to make this possible.

Screengrab

CleanShot 2024-04-27 at 19 43 25@2x

How to test

  1. Launch the app with the test Rivet project below
  2. Switch executor to Node
  3. Run entire graph
  4. Observe that fetched image (of my cat 😼) should show up in the ImageNode, proving that the feature works

Test Rivet project

version: 4
data:
  attachedData:
    trivet:
      testSuites: []
      version: 1
  graphs:
    q8Tc5Q-34WB8e-loWqoON:
      metadata:
        description: ""
        id: q8Tc5Q-34WB8e-loWqoON
        name: Untitled Graph
      nodes:
        '[Ux7TlwMXhLmKxUjdiDuIc]:httpCall "Http Call"':
          data:
            body: ""
            errorOnNon200: true
            headers: ""
            method: GET
            url: https://github.com/liaujianjie.png
          outgoingConnections:
            - binary->"Image" owgnZUkpDj7Ik93OtyOGd/data
          visualData: 497/390/280/3//
        '[owgnZUkpDj7Ik93OtyOGd]:image "Image"':
          data:
            mediaType: image/png
            useDataInput: true
            useMediaTypeInput: false
          visualData: 1029/376/484.1567767955994/7//
  metadata:
    description: ""
    id: _A_qCLGKHitAnTSshRby2
    title: Untitled Project
  plugins: []
abrenneke commented 2 months ago

This is great @liaujianjie! I was thinking, what if there were a setting toggle in the node instead of both binary and text outputs? I can't really think of a case where you'd want both the binary and text output of an HTTP call, so instead we can just let the use choose which type the output of the node should be in the settings panel. That makes the node maybe simpler to use?

liaujianjie commented 2 months ago

This is great @liaujianjie! I was thinking, what if there were a setting toggle in the node instead of both binary and text outputs? I can't really think of a case where you'd want both the binary and text output of an HTTP call, so instead we can just let the use choose which type the output of the node should be in the settings panel. That makes the node maybe simpler to use?

Definitely agree with this approach, thought I'd need some pointers on how I should do that since I'm still pretty new to Rivet.

An alternative would be to separate these into 2 PRs? So we have this PR for adding support for for the binary port on HttpCallNode, then a second PR for adding the switch/select to choose between JSON/binary/text. This way we can also keep the scope of this PR small.

Thoughts?

abrenneke commented 2 months ago

That would mean maybe two releases and backwards incompatible stuff, so probably better to nail this the first time. Should be pretty easy, just checking this.data.isBinary in both the output definitions function and the main process body. No more difficult than the stuff you've already written.

liaujianjie commented 2 months ago

What about JSON outputs? Shouldn't the output type be one of:

  1. string,
  2. JSON, or
  3. binary?
liaujianjie commented 1 month ago

@abrenneke I've went ahead to implement it such that the user can either switch between:

  1. binary
  2. string or JSON

The option to select between the two is just below the body:

CleanShot 2024-05-03 at 00 14 59@2x

What do you think?

abrenneke commented 1 month ago

@all-contributors please add @liaujianjie for code

allcontributors[bot] commented 1 month ago

@abrenneke

I've put up a pull request to add @liaujianjie! :tada: