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

CNF-13067: O-RAN V3 Rest Api: Status Notification #71

Merged
merged 1 commit into from
Jul 15, 2024

Conversation

jzding
Copy link
Member

@jzding jzding commented Jul 4, 2024

  • Remove local store for pubsub: sub.json. Now access to sub info only through SubscriberAPI/store.
  • Update UriLocation to use service name.
  • Remove subject from Cloud Event
  • Remove datacontenttype from Cloud Event
  • Reconstruct configMap for subscription info
  • Fix response code in subscription failure cases.
  • Added unit tests

More details in doc https://docs.google.com/document/d/1aIgGWhFDDz9aI7mIZm_nlkb99UrV7hv5gS5m_XShCt4/edit

@jzding jzding force-pushed the oran-api branch 5 times, most recently from c30f8a7 to 3db84e4 Compare July 9, 2024 02:12
@@ -67,26 +67,19 @@ func (s *Server) createSubscription(w http.ResponseWriter, r *http.Request) {
localmetrics.UpdateSubscriptionCount(localmetrics.FAILCREATE, 1)
return
}
if subExists, ok := s.pubSubAPI.HasSubscription(sub.GetResource()); ok {
clientIDs := s.subscriberAPI.GetClientIDByResource(sub.GetResource())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why Plural clientID's?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be one clientID for the resource

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This returns clientIDs []uuid.UUID. I did not change the logic.
https://github.com/redhat-cne/sdk-go/blob/main/v1/subscriber/subscriber.go#L254

ce.SetDataContentType(cloudevents.ApplicationJSON)
ce.SetSpecVersion(cloudevents.VersionV03)
ce := cloudevents.NewEvent(cloudevents.VersionV1)
ce.SetSpecVersion(cloudevents.VersionV1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not setting content type?

Copy link
Member Author

@jzding jzding Jul 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In O-RAN V3.0 spec, Event Data Model (chapter 7.2.2) does not have datacontenttype and subject, so I removed them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed ce.SetSubject() from simplehttp. I think we are fine here. The subject is not actually used in internal API. The ProcessOutChannel() only uses Status and Action.
https://github.com/redhat-cne/cloud-event-proxy/blob/main/cmd/main.go#L219

@jzding jzding force-pushed the oran-api branch 2 times, most recently from c297afc to 00cbdce Compare July 10, 2024 02:36
@jzding jzding requested a review from aneeshkp July 10, 2024 12:50
@aneeshkp
Copy link
Member

Please add description to this PR

@@ -49,7 +49,7 @@ type Subscriber struct {
SubStore *store.PubSubStore `json:"subStore" omit:"empty"`
// EndPointURI - A URI describing the subscriber link .
// +required
EndPointURI *types.URI `json:"endPointURI" omit:"empty"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible the internal API direct call failure in older version ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no. tested with old version. The subscription was successful.

@@ -84,8 +84,8 @@ func (s *Subscriber) FailedCount() int {
// String returns a pretty-printed representation of the Event.
func (s *Subscriber) String() string {
b := strings.Builder{}
b.WriteString(" EndPointURI: " + s.GetEndPointURI() + "\n")
b.WriteString(" ID: " + s.GetClientID().String() + "\n")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no. tested with old version. The subscription was successful.

@jzding
Copy link
Member Author

jzding commented Jul 11, 2024

Please add description to this PR

done

@jzding jzding force-pushed the oran-api branch 2 times, most recently from b1356a1 to becd572 Compare July 12, 2024 22:33
Signed-off-by: Jack Ding <jackding@gmail.com>
@jzding jzding merged commit eecb75b into redhat-cne:main Jul 15, 2024
2 checks passed
jzding added a commit to jzding/rest-api that referenced this pull request Aug 29, 2024
Signed-off-by: Jack Ding <jackding@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants