Skip to content

Comments

fix(localega-tsd-proxy): fixes calls to LS login per request#687

Open
joshbaskaran wants to merge 10 commits intomainfrom
686-implement-caching-of-ls-login
Open

fix(localega-tsd-proxy): fixes calls to LS login per request#687
joshbaskaran wants to merge 10 commits intomainfrom
686-implement-caching-of-ls-login

Conversation

@joshbaskaran
Copy link
Contributor

fixes #686

@joshbaskaran joshbaskaran changed the title feat: add oauth2 resource server dependency fix: fixes calls to LS login server per request Jan 16, 2026
@joshbaskaran joshbaskaran self-assigned this Jan 16, 2026
@joshbaskaran joshbaskaran requested a review from kjellp January 16, 2026 15:05
Copy link
Contributor

@kjellp kjellp left a comment

Choose a reason for hiding this comment

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

Looks good so far to me. Do we implement testing of the feature here? And make cache expire time configurable?

@joshbaskaran
Copy link
Contributor Author

Looks good so far to me. Do we implement testing of the feature here? And make cache expire time configurable?

I can make it configurable here, but I think the test should be a separate thing that's better done at the e2e scope.

@joshbaskaran joshbaskaran force-pushed the 686-implement-caching-of-ls-login branch from d17d2dd to a715e90 Compare January 19, 2026 07:29
@joshbaskaran joshbaskaran requested a review from kjellp January 19, 2026 08:24
@joshbaskaran joshbaskaran changed the title fix: fixes calls to LS login server per request fix(localega-tsd-proxy): fixes calls to LS login per request Jan 19, 2026
@kjellp kjellp force-pushed the 686-implement-caching-of-ls-login branch from a715e90 to b222abe Compare January 19, 2026 10:35
@kjellp
Copy link
Contributor

kjellp commented Jan 19, 2026

Excellent with that quick update to make caching parameters configurable @joshbaskaran :)

To include it at e2e level testing is an option, but was unsure if it really belonged there, as this may become a "costly" test, and belong more at build level, than for every e2e test run for any change of the mono-repo in the future?

But for now we can aim at test it manually on egadev with an e2e run there (using live LS-login tokens), before we approve and merge it. We need to design how to test it, with both positive and negative tests though.

@joshbaskaran
Copy link
Contributor Author

Excellent with that quick update to make caching parameters configurable @joshbaskaran :)

To include it at e2e level testing is an option, but was unsure if it really belonged there, as this may become a "costly" test, and belong more at build level, than for every e2e test run for any change of the mono-repo in the future?

But for now we can aim at test it manually on egadev with an e2e run there (using live LS-login tokens), before we approve and merge it. We need to design how to test it, with both positive and negative tests though.

Thanks! :)
E2e is indeed quite costly. I might be able to write a rudimentary test to see how many HTTP calls to the OIDC server are made on multiple requests. I'll look a bit more into this!

@joshbaskaran
Copy link
Contributor Author

@kjellp added the unit test and it seems to do its job! Ready for a review and merge :)

@joshbaskaran joshbaskaran force-pushed the 686-implement-caching-of-ls-login branch 2 times, most recently from 7672db3 to 214a647 Compare February 18, 2026 08:28
Copy link
Contributor

@pavelvazquez pavelvazquez left a comment

Choose a reason for hiding this comment

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

E2E in egadev passed and manual upload completed successfully

@kjellp kjellp force-pushed the 686-implement-caching-of-ls-login branch from ab3039b to 9e6b2cb Compare February 19, 2026 06:56
@joshbaskaran joshbaskaran force-pushed the 686-implement-caching-of-ls-login branch from 9e6b2cb to b08b958 Compare February 20, 2026 19:14
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.

Implement caching of LS Login public key for verifying LS login tokens in the Proxy service

4 participants