Skip to content

Document the trusted port for callbacks and docs#52

Open
vrajmohan wants to merge 3 commits intomainfrom
adr-public-endpoints
Open

Document the trusted port for callbacks and docs#52
vrajmohan wants to merge 3 commits intomainfrom
adr-public-endpoints

Conversation

@vrajmohan
Copy link
Contributor

No description provided.

mpidcock
mpidcock approved these changes Sep 14, 2023
Comment on lines 26 to 29
| Port | Use | Security | Path |
|-|-|-|-|
| 8080 | Messaging API | IP Filtering & BasicAuth shared password | https://app-54372.on-aptible.com |
| [8086](/src/main/resources/application.properties) | Messaging Provider Callbacks & API Documentation | Public w/ Provider Signature varification | https://staging.messaging.cfa-platforms.org/public |
Copy link
Member

Choose a reason for hiding this comment

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

@vrajmohan I added this table to make the info easier to see at a glance. I know it is subject to change, especially the port, but it seems unlikely to me that it will.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO, the ADR does not need implementation details like a README or INSTALLING. I expect the engineers to read the code to get those details. I would rather focus the ADR on pros and cons of the decisions made.

Copy link
Member

Choose a reason for hiding this comment

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

I added the table to make it easier to visually process the information, that there are two ports with different security measures and different urls. The actual ports and urls are not relevant, but I think seeing them broken down in a table helps it click. I agree that this should not be the "source" for this info, and maybe it is risky to have it shown here, but I think it helps clarify the impact of this ADR.

Copy link
Member

Choose a reason for hiding this comment

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

Updated the table to remove implementation details

@mpidcock mpidcock removed their assignment Sep 15, 2023
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