jesusdalvarado / rapids-v1

0 stars 0 forks source link

Missing cloudevent source argument #4

Open erik-ekberg-sg opened 3 years ago

erik-ekberg-sg commented 3 years ago

A source property is required by default to build a cloudevent. Usually this defaults to window.location.href and if that is not available some other unique identifier is used as the reverse DNS of a service (e.g. com.my-domain.www; cloudevent source spec).

I would recommend adding that required source attribute to the emit interface.

const emit = async ({ data, type, source: typeof window !== 'undefined' ? window.location.href : 'my-default-source' }) => {
  if (!data) { throw new Error('"data" is required') }
  if (!type) { throw new Error('"type" is required') }
  if (!source) { throw new Error('"source" is required') }

  console.log(`Emitting ${type} data: ${data}`)
  const lambda = new Lambda({})
  await lambda.invoke({
    cloudevent: new Cloudevent({ data, type, source }),
    functionName: 'rapids-v1-hydrator-v0',
  })
}
jesusdalvarado commented 3 years ago

@erik-ekberg-sg I think we should add the source as an optional field (not required at the emit level), this way we can have the flexibility to set it, but at the same time the simplicity of the interface that only requires the type and data (or even just the type when no data is needed). The source is a field that helps us to track where the event is coming from, so setting it as optional and adding a default value should do the trick.

erik-ekberg-sg commented 3 years ago

@jesusdalvarado, agreed. A source field is required at the cloudevent level but we can easily set a default source value to pass to the cloudevent at the emit level. Per the above, I recommend

const emit = async ({ ..., source: typeof window !== 'undefined' ? window.location.href : 'my-default-source' })

but replace 'my-default-source' with likely process.env.JESUS_EMIT_SOURCE or process.env.JESUS_MILL_CLOUDEVENTS_SOURCE.