Skip to content

Refactor translation interceptor#87

Merged
pglass merged 3 commits intomainfrom
pglass/translation-refactor
Jun 13, 2025
Merged

Refactor translation interceptor#87
pglass merged 3 commits intomainfrom
pglass/translation-refactor

Conversation

@pglass
Copy link
Copy Markdown
Contributor

@pglass pglass commented Jun 4, 2025

What was changed

  • Rename/replace NamespaceNameTranslator interceptor with more generic TranslationInterceptor
  • TranslationInterceptor supports a list of basic Translators and we can more easily add additional translators (like for cluster name translation)

Why?

Checklist

  1. Closes

  2. How was this tested:

  1. Any docs updates needed?

@pglass pglass requested a review from a team as a code owner June 4, 2025 17:42
@pglass pglass requested a review from hehaifengcn June 4, 2025 17:43
type (
TranslationInterceptor struct {
logger log.Logger
translators []Translator
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Basic idea is we could have a translator in this list for namespace name and another for cluster name translation

Comment thread proxy/proxy.go
)
}

if len(translators) > 0 {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

instead of check len(translators), can we just move interceptor.NewTranslationInterceptor up and check if tr != nil?

Copy link
Copy Markdown
Contributor Author

@pglass pglass Jun 4, 2025

Choose a reason for hiding this comment

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

Plan is to also support adding cluster name translation to the list of translators - see subsequent PR: https://github.com/temporalio/s2s-proxy/pull/88/files#diff-1d74870266949f1927a500e5f0002617239d3e3cd9bcceee6be014738e5e0f2bR56-R74

So only want to call interceptor.NewTranslationInterceptor if translators is non empty.

Base automatically changed from pglass/translator-benchmark to main June 4, 2025 19:21
@pglass pglass enabled auto-merge (squash) June 13, 2025 19:18
@pglass pglass merged commit f10400b into main Jun 13, 2025
5 checks passed
@pglass pglass deleted the pglass/translation-refactor branch June 13, 2025 19:20
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