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

Update ui-server to use the new protos #1775

Merged
merged 18 commits into from
Jan 29, 2024
Merged

Update ui-server to use the new protos #1775

merged 18 commits into from
Jan 29, 2024

Conversation

tdeebswihart
Copy link
Contributor

@tdeebswihart tdeebswihart commented Dec 14, 2023

Description & motivation 💭

This updates the UI server to use our new google/protobuf-compiled proto code. This introduces a breaking change to our JSON formatting: all proto enums are now in SCREAMING_SNAKE_CASE rather than camelCase.

This PR (and branch) is just to get y'all what you need to update the UI itself and this should not be merged on its own.

Screenshots (if applicable) 📸

N/A

Design Considerations 🎨

N/A

Testing 🧪

How was this tested 👻

No tests were added and all tests have likely broken. This PR is a starting point upon which the UI itself can be updated. I've done some very basic manual testing to ensure the API returns what we expect it to, like the new SCREAMING_SNAKE_ENUMS:

$ xh localhost:8081/api/v1/namespaces
HTTP/1.1 200 OK
Content-Length: 1349
Content-Type: *
...
{
  "namespaces": [
    {
      "namespaceInfo": {
        "name": "temporal-system",
        "state": "NAMESPACE_STATE_REGISTERED", # NEW ENUM FORMAT
        "description": "Temporal internal system namespace",
...
  • Manual testing
  • E2E tests added
  • Unit tests added

Steps for others to test: 🚶🏽‍♂️🚶🏽‍♀️

  1. Update the UI to handle SCREAMING_SNAKE_CASE enums
  2. Run existing tests

Checklists

Draft Checklist

  • Need to fix query api params

Merge Checklist

Issue(s) closed

Docs

Any docs updates needed?

Copy link

vercel bot commented Dec 14, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
holocene ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 29, 2024 6:23pm

The actual error we hit was
```
2024/01/04 13:08:28 Failed to marshal *status.Status: temporalproto:
google.protobuf.Any: unable to resolve
"type.googleapis.com/temporal.api.errordetails.v1.NotFoundFailure":
proto: not found`
```
@Alex-Tideman Alex-Tideman marked this pull request as ready for review January 22, 2024 21:21
@Alex-Tideman Alex-Tideman marked this pull request as draft January 22, 2024 21:24
@Alex-Tideman Alex-Tideman self-requested a review January 22, 2024 21:45
Copy link
Contributor

@laurakwhit laurakwhit left a comment

Choose a reason for hiding this comment

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

Seeing undefined for this column ⬇️ Once that's fixed I say merge it 🚀

Screenshot 2024-01-29 at 11 06 38 AM

src/lib/services/namespaces-service.ts Show resolved Hide resolved
…ests to cover object/array inputs, and show empty string for ParentNamespaceID in workflow cell.
@Alex-Tideman Alex-Tideman merged commit 44d4721 into main Jan 29, 2024
9 checks passed
@Alex-Tideman Alex-Tideman deleted the tds/google-protobuf branch January 29, 2024 18:27
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.

3 participants