Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: regulation api support for Universal Analytics #2632
feat: regulation api support for Universal Analytics #2632
Changes from 27 commits
dfddb48
f4a67e5
9af81ec
fec7530
8b95415
7609b3f
39550a0
d77c224
1db26f3
d3665d5
de288dc
6b1f817
616839a
8fcec3d
1f59039
8083f7c
bcd80e3
21f8113
ddefc36
8878e35
34564ab
a0cb000
298050d
cb76fce
4cac21e
c3d5634
5a20251
a25068c
b46d62d
51de4a9
2e677f8
6ba9d19
d948275
94ea214
45a473a
5a48d74
fa062da
464e11f
47272cd
4d8a6ce
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We seem to need too much worker-specific code for managing an oauth flow. This could be an indication that we are missing the proper abstractions for a truly generic oauth service.
Why do we need all this custom code here and why does it need to differ from the code that router uses? Can we achieve the same result through a generic oath http interceptor, reused with minimal effort wherever we have a need for performing oauth in our requests?
Our goal is to perform an http request using an oauth token & retry this request if it fails due to an expired oauth token after refreshing it and all this transparently from the caller (router/regulation-worker). IMO this is a job for a generic interceptor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes sense to think of the oauth flow as an interceptor. IMHO it seems like the ideal approach for this kind of feature
Making changes to make sure we use this as an interceptor doesn't fit in our current plan. I have outlined a couple of reasons for it
rudder-server
. Keeping this in mind & the changes that we need to do in order for us to make sure the interceptor approach is being used will be time-takingBut we will make sure to keep this thought in mind while we work on OAuth Flow movement itself. Please let me know if this makes sense