Skip to content
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

Delivery Format: add the format field to the Delivery type #8073

Closed
Cali0707 opened this issue Jul 4, 2024 · 7 comments · Fixed by #8080
Closed

Delivery Format: add the format field to the Delivery type #8073

Cali0707 opened this issue Jul 4, 2024 · 7 comments · Fixed by #8080
Assignees
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature-request triage/accepted Issues which should be fixed (post-triage)

Comments

@Cali0707
Copy link
Member

Cali0707 commented Jul 4, 2024

Problem
To support more destinations of cloudevents (which may require a specific event format) we need to add the format field to the DeliverySpec type which is embedded into many of our types.

Persona:
Which persona is this feature for?

Exit Criteria
The format field is present in the DeliverySpec type, and is validated to be one of:

  1. nil
  2. structured
  3. binary
  4. ingress

Time Estimate (optional):
How many developer-days do you think this may take to resolve? 1

Additional context (optional)
Add any other context about the feature request here.

  • The parent issue is Delivery Format: support specifying the event format #8057
  • The delivery spec is here:
    type DeliverySpec struct {
    // DeadLetterSink is the sink receiving event that could not be sent to
    // a destination.
    // +optional
    DeadLetterSink *duckv1.Destination `json:"deadLetterSink,omitempty"`
    // Retry is the minimum number of retries the sender should attempt when
    // sending an event before moving it to the dead letter sink.
    // +optional
    Retry *int32 `json:"retry,omitempty"`
    // Timeout is the timeout of each single request. The value must be greater than 0.
    // More information on Duration format:
    // - https://www.iso.org/iso-8601-date-and-time-format.html
    // - https://en.wikipedia.org/wiki/ISO_8601
    //
    // Note: This API is EXPERIMENTAL and might break anytime. For more details: https://github.com/knative/eventing/issues/5148
    // +optional
    Timeout *string `json:"timeout,omitempty"`
    // BackoffPolicy is the retry backoff policy (linear, exponential).
    // +optional
    BackoffPolicy *BackoffPolicyType `json:"backoffPolicy,omitempty"`
    // BackoffDelay is the delay before retrying.
    // More information on Duration format:
    // - https://www.iso.org/iso-8601-date-and-time-format.html
    // - https://en.wikipedia.org/wiki/ISO_8601
    //
    // For linear policy, backoff delay is backoffDelay*<numberOfRetries>.
    // For exponential policy, backoff delay is backoffDelay*2^<numberOfRetries>.
    // +optional
    BackoffDelay *string `json:"backoffDelay,omitempty"`
    // RetryAfterMax provides an optional upper bound on the duration specified in a "Retry-After" header
    // when calculating backoff times for retrying 429 and 503 response codes. Setting the value to
    // zero ("PT0S") can be used to opt-out of respecting "Retry-After" header values altogether. This
    // value only takes effect if "Retry" is configured, and also depends on specific implementations
    // (Channels, Sources, etc.) choosing to provide this capability.
    //
    // Note: This API is EXPERIMENTAL and might be changed at anytime. While this experimental
    // feature is in the Alpha/Beta stage, you must provide a valid value to opt-in for
    // supporting "Retry-After" headers. When the feature becomes Stable/GA "Retry-After"
    // headers will be respected by default, and you can choose to specify "PT0S" to
    // opt-out of supporting "Retry-After" headers.
    // For more details: https://github.com/knative/eventing/issues/5811
    //
    // More information on Duration format:
    // - https://www.iso.org/iso-8601-date-and-time-format.html
    // - https://en.wikipedia.org/wiki/ISO_8601
    //
    // +optional
    RetryAfterMax *string `json:"retryAfterMax,omitempty"`
    }
@Cali0707
Copy link
Member Author

Cali0707 commented Jul 4, 2024

/good-first-issue

Copy link

knative-prow bot commented Jul 4, 2024

@Cali0707:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

/good-first-issue

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@knative-prow knative-prow bot added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. labels Jul 4, 2024
@Cali0707 Cali0707 closed this as completed by moving to Ready in Eventing Delivery Format Jul 4, 2024
@Cali0707 Cali0707 closed this as completed by moving to Ready in Eventing Delivery Format Jul 4, 2024
@Cali0707 Cali0707 reopened this Jul 4, 2024
@Cali0707
Copy link
Member Author

Cali0707 commented Jul 4, 2024

Whoops sorry @EraKin575 I think I messed up the project automation there so this got closed accidentally, I've reopened it now - feel free to ask for help if you need it as you work on it!

@EraKin575
Copy link
Contributor

thanks! @Cali0707 . I got really confused there

@EraKin575
Copy link
Contributor

EraKin575 commented Jul 4, 2024

hi @Cali0707 ! is #8057 a prerequisite to this?

@Cali0707
Copy link
Member Author

Cali0707 commented Jul 4, 2024

hi @Cali0707 ! is #8057 a prerequisite to this?

Nope, this is a sub task for #8057 - this can be worked on now

@EraKin575
Copy link
Contributor

@Cali0707 this field just needs to be added in the struct and validate in Validate function, right? Nothing else to do?

@EraKin575 EraKin575 mentioned this issue Jul 5, 2024
5 tasks
@pierDipi pierDipi moved this from Ready to Done in Eventing Delivery Format Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature-request triage/accepted Issues which should be fixed (post-triage)
Projects
Development

Successfully merging a pull request may close this issue.

2 participants