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.

Sign up for GitHub

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

Jump to bottom

Finish galley servantification #4018

Merged
merged 18 commits into from
Apr 30, 2024
Merged

Finish galley servantification #4018

merged 18 commits into from
Apr 30, 2024
+346 −639

Conversation

pcapriotti
Copy link
Contributor

@pcapriotti pcapriotti commented Apr 25, 2024 •
edited
Loading

Servantify internal endpoints and one final public endpoint. This supersedes #3997.

https://wearezeta.atlassian.net/browse/WPB-1223

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@mdimjasevic mdimjasevic self-requested a review April 25, 2024 09:38
@pcapriotti pcapriotti mentioned this pull request Apr 25, 2024
Closed
2 tasks
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Apr 25, 2024
mdimjasevic
mdimjasevic reviewed Apr 25, 2024
View reviewed changes
Copy link
Contributor

@mdimjasevic mdimjasevic left a comment

Choose a reason for hiding this comment

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

I am still reviewing, but maybe you wanted to know of this TODO left in the code before I am done with the whole PR.

services/galley/src/Galley/API/Clients.hs Outdated Show resolved Hide resolved
@mdimjasevic
Copy link
Contributor

Would it make sense to rename the Galley.API.Public.Servant module to just Galley.API.Public? We don't have to emphasize it's a Servant module because all are once this PR is merged.

mdimjasevic
mdimjasevic approved these changes Apr 25, 2024
View reviewed changes
Copy link
Contributor

@mdimjasevic mdimjasevic left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Modulo the TODO note, I think this is good to go.

@pcapriotti pcapriotti force-pushed the pcapriotti/servantify-galley-internal branch 2 times, most recently from 79e28fc to f24f0c2 Compare April 29, 2024 07:28
@pcapriotti
Servantify get-team-members
d727e52
@pcapriotti
Servantify get-team-id
3a80ab3
@pcapriotti
Servantify test-get-clients
8c40ad2
@pcapriotti
Servantify test-add-client
2ad47f7
@pcapriotti
Servantify test-delete-clients
05f1be7
@pcapriotti
Servantify add-service
6706390
@pcapriotti
Servantify delete-service
9668af3
@pcapriotti
Servantify add-bot
333884c
@pcapriotti
Servantify delete-bot
cd7e75d
@pcapriotti
Servantify put-custom-backend
2d0bb79
@pcapriotti
Servantify delete-custom-backend
53a1f9e
@pcapriotti
Servantify get-bot-conversation
250d015
@pcapriotti
Remove wai-routing from galley
a3c92fd
@pcapriotti
Remove WaiRoutes effect from galley
ae8439e
@pcapriotti
Add CHANGELOG entry
bf033c7
@pcapriotti
Update nix packages
86eedfc
@pcapriotti
Refactor getClients
b2651f1
@pcapriotti
Add fallback endpoint for compatibility
e6b17ae 
Removing wai-routing's Raw endpoint in galley's servant tree changes the
error responses for endpoints with recoverable failures (i.e. `Fail` route
results in Servant).

This commit restores compatibility by adding a fallback endpoint that
always responds with 404.
@pcapriotti pcapriotti force-pushed the pcapriotti/servantify-galley-internal branch from f24f0c2 to e6b17ae Compare April 29, 2024 09:45
@pcapriotti pcapriotti merged commit 4c82fd6 into develop Apr 30, 2024
8 checks passed
@pcapriotti pcapriotti deleted the pcapriotti/servantify-galley-internal branch April 30, 2024 07:38
@zebot zebot mentioned this pull request May 21, 2024
Merged
@echoes-hq echoes-hq bot added the echoes: technical-roadmap/technical-debt Changes intended at mitigating risks label Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mdimjasevic mdimjasevic mdimjasevic approved these changes

Assignees
No one assigned
Labels
echoes: technical-roadmap/technical-debt Changes intended at mitigating risks ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pcapriotti @mdimjasevic @zebot