ornj / hypernova-python

Python client for Hypernova https://github.com/airbnb/hypernova
MIT License
11 stars 2 forks source link

Allow job/token name to be different than component name? #12

Open imricardoramos opened 4 years ago

imricardoramos commented 4 years ago

Should the lib be able to send a batch request for the same component, but with different props? Eg, I was trying to do the following request:

{
  "Job1": {
    "name": "MyComponent",
    "data": {
      "title": "Foo",
      "content": "foo"
    }
  },
  "Job2": {
    "name": "MyComponent",
    "data": {
      "title": "Bar",
      "content": "bar"
    }
  }
}

However, I can only do this:

html = renderer.render({
    'MyComponent': {
        'title': 'Foo',
        'content': 'foo'
    },
    'MyComponent': {
        'title': 'Bar',
        'content': 'bar'
    }
})

which makes the first job be overridden by the second job because they have same component name (https://github.com/ornj/hypernova-python/blob/master/hypernova/__init__.py#L53).

I've taken a quick look at the nodejs and the ruby clients, and it seems they do it differently:

In Ruby (https://github.com/airbnb/hypernova-ruby/blob/master/lib/hypernova/batch.rb#L45), they make an array of jobs, and then make a hash with the keys being the index of the job in the array.

In Node(https://github.com/airbnb/hypernova-node/blob/master/src/index.js#L89), they use the component name as the job name (as in this lib)

marteinn commented 4 years ago

Hi @imricardoramos and thanks for the bug report (and additional research)! I will try to find the time the next couple of days to recreate the issue, will get back to you after that. Stay tuned :)

marteinn commented 4 years ago

Thanks for your patience @imricardoramos. To first answer your question: No, it is currently not supported to retrieve more then one component per type in a batch request.

To add support for this we most likely need to change the internal data representation of the job queue to a list and introduce a feature for generating job ids.

Currently you have two choices:

Going forward, I think taking a hard look at how the internal data representation are organized and how we can enable this functionality without breaking any api while keeping a similar api with the other client libraries out there are the way to go. This will not be solved now, but I will keep this ticket open and add proper tags to this issue and hopefully revisit in the future. If you have any ides, PR:s are very welcome.