Skip to content
This repository has been archived by the owner on Apr 25, 2023. It is now read-only.

Add paging support for Github requests #764

Closed
wants to merge 1 commit into from

Conversation

gregturn
Copy link
Contributor

@gregturn gregturn commented Oct 3, 2017

No description provided.

@gregturn gregturn force-pushed the paging-2 branch 2 times, most recently from 1bbbd0a to c140c8e Compare November 27, 2017 19:20
@gregturn
Copy link
Contributor Author

@bclozel I rebased this PR against latest master.

Also, I tried to configure Eclipse Code Formatter, but it still won't apply imports in the proper order.

@@ -17,6 +18,11 @@
return operations.getForObject(url, clazz);
}

@Cacheable(value = CACHE_NAME, key = "#url")
Copy link
Contributor

Choose a reason for hiding this comment

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

This won't work since we're already using that key to cache objects of types T. So fetching from the cache using an URL, you can get the wrong thing depending on the method.

Also, ResponseEntity is not serializable and that won't work with the default JDK serializer used by the redis cache manager. See CacheConfig for the configuration used for other types.

@bclozel
Copy link
Contributor

bclozel commented Dec 1, 2017

I should review the current caching infrastructure because this is not making it easier to leverage more caching.
Note that I can't merge this right now as this throws the following as soon as we're trying to cache data:

ERROR 14 --- [nio-8080-exec-4] o.a.c.c.C.[.[.[/].[dispatcherServlet] : 
Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception [Request processing failed;
nested exception is org.springframework.data.redis.serializer.SerializationException: Cannot serialize; 
nested exception is org.springframework.core.serializer.support.SerializationFailedException: Failed to serialize object using DefaultSerializer;
nested exception is java.lang.IllegalArgumentException: DefaultSerializer requires a Serializable payload but received an object of type [org.springframework.http.ResponseEntity]] with root cause

@gregturn
Copy link
Contributor Author

gregturn commented Dec 15, 2017

Digging further in, I actually need the ResponseEntity version of this RestTemplate's response because I need access to the content as well as the headers (the Link header to be precise). Given all that, it would appear that flagging this as cacheable isn't the right solution. I'll amend the PR.

@gregturn
Copy link
Contributor Author

gregturn commented Jan 5, 2018

@bclozel Friendly ping.

@bclozel
Copy link
Contributor

bclozel commented Apr 7, 2020

This part has been moved to sagan-renderer and supports now paging for requests to GitHub.
Thanks!

@bclozel bclozel closed this Apr 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants