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

Revamp content-based-message-routing tutorial #8223

Conversation

dulajdilshan
Copy link
Contributor

Purpose

$title

Checklist

  • Page addition

    • Add permalink to pages.
  • Page removal

    • Remove entry from corresponding left nav YAML file.
    • Add redirect_from on the alternative page.
    • If no alternative page, add redirection on the redirections.js file.
  • Page rename

    • Add front-matter redirect_from.
    • Add front-matter redirect_to: (if applicable).
  • Page restrcuture

    • Add permalink to pages.
    • Add front-matter redirect_from.
    • Add front-matter redirect_to: (if applicable).

@dulajdilshan dulajdilshan changed the title Revamp "content-based-message-routing" tutorial Revamp content-based-message-routing tutorial Nov 9, 2023
@mindula
Copy link
Contributor

mindula commented Nov 10, 2023

The HospitalId enum is missing in the define_record_types GIF.

Co-authored-by: Mindula Rowel <61020198+mindula@users.noreply.github.com>
Copy link
Contributor

@mindula mindula left a comment

Choose a reason for hiding this comment

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

LGTM

@dulajdilshan dulajdilshan merged commit cf84223 into ballerina-platform:integration-tutorials Nov 10, 2023
1 check passed
@dulajdilshan dulajdilshan deleted the revamp-content-based-routing branch November 10, 2023 11:59
> final http:Client clemencyEP = new ("http://localhost:9090/clemency/categories");
> final http:Client pinevalleyEP = new ("http://localhost:9090/pinevalley/categories");
>```
4. Generate record types corresponding to the payloads from the hospital backend services by providing samples of the expected JSON payloads.
Copy link
Member

Choose a reason for hiding this comment

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

There's only one backend call.

So

  • payloads -> payload
  • services -> service

Comment on lines +167 to +168

type ReservationRequest record {
Copy link
Member

Choose a reason for hiding this comment

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

Let's move this out and have a similar point for request payload.

@@ -243,77 +257,13 @@ Follow the instructions given in this section to develop the service.

- Define the `hospitalEP` variable. Later, the relevant client is assigned to this variable based on the `hospital_id` value.
Copy link
Member

Choose a reason for hiding this comment

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

  • Define an http:Client variable named hospitalEP. Later, the relevant client is assigned to this variable based on the hospital_id value.

}
}
```

- Make a `POST` request to the backend hospital service to make the reservation. The `category` value is used as a path parameter.
Copy link
Member

Choose a reason for hiding this comment

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

  • Make a POST request to the relevant backend hospital service to make the reservation. The category value is used as a path parameter.

```ballerina
{patient, doctor, ...reservationRequest}
```

- Use the `is` check to check whether the response is a `ReservationResponse` and return it as is (i.e., reservation successful).
Copy link
Member

Choose a reason for hiding this comment

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

  • Use the is check to check whether the response is a ReservationResponse record, which would indicate a successful reservation. If the response is a ReservationResponse record, return the record from the resource method to send a 201 Created response with the ReservationResponse value as the payload.


return <http:InternalServerError> {body: resp.message()};
```
- If the response is not a `ReservationResponse`, log the failure at `ERROR` level. Return a "NotFound" response if the response is an `http:ClientRequestError`, or an "InternalServerError" response if the response is an `http:ServerError`.
Copy link
Member

Choose a reason for hiding this comment

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

"NotFound" -> http:NotFound

Fix "InternalServerError" too.


return <http:InternalServerError> {body: resp.message()};
```
- If the response is not a `ReservationResponse`, log the failure at `ERROR` level. Return a "NotFound" response if the response is an `http:ClientRequestError`, or an "InternalServerError" response if the response is an `http:ServerError`.
Copy link
Member

Choose a reason for hiding this comment

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

If the response is not a ReservationResponse record (i.e., http:ClientError), which indicates that the request failed, log the failure at ERROR level.

type ReservationRequest record {
Patient patient;
string doctor;
HospitalId hospital_id;
Copy link
Member

Choose a reason for hiding this comment

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

The record isn't generated with HospitalId as the type, right? Let's add a note to mention updating this.

HospitalId hospital_id;
string hospital;
string appointment_date;
};
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't Patient get generated twice? Can add a note asking to remove one if it matches an already existing record too.

Copy link
Member

Choose a reason for hiding this comment

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

This could even be a tooling improvement. If we can check if there's a record that's the same type and same name, we avoid adding a new one.

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.

None yet

3 participants