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

Move RequestManager to a separate package and make it more generic #2155

Closed
2 of 3 tasks
provokateurin opened this issue Jun 10, 2024 · 4 comments
Closed
2 of 3 tasks
Labels
package: neon_framework refactoring Something that needs to be refactored

Comments

@provokateurin
Copy link
Member

provokateurin commented Jun 10, 2024

@provokateurin provokateurin added package: neon_framework refactoring Something that needs to be refactored labels Jun 10, 2024
@Leptopoda Leptopoda self-assigned this Jun 12, 2024
@provokateurin
Copy link
Member Author

I already have a bunch of this work locally and some is read to merge, @Leptopoda can I assign me to this issue?

@Leptopoda
Copy link
Member

I think we should implement the cache without any serialization logic.
I.e. without the request manager. Similar to #2165, by just adding a cache interceptor.

Ideally the cache does work directly with http.Requests and http.Responses. The client that does the deserialization shouldn't care whether the data is fetched from the cache or upstream. If needed I'd rather append a header to the http.Response signaling that it was cached (I think that's what the age header is for https://www.rfc-editor.org/rfc/rfc7234#section-5.1).

That way the caching would be separate from the request manager itself where the manager would only be responsible for making the requests (maybe with different priorities ...)

@provokateurin
Copy link
Member Author

Yes that's my plan eventually. With #2237 I could also move the whole logic into the cache, like I said in the initial comment on the PR.

@provokateurin
Copy link
Member Author

@Leptopoda we can probably close this as we want to get rid of it entirely at some point? The other stuff now already lives in neon_http_client.

@Leptopoda Leptopoda closed this as not planned Won't fix, can't repro, duplicate, stale Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: neon_framework refactoring Something that needs to be refactored
Projects
None yet
Development

No branches or pull requests

2 participants