-
Notifications
You must be signed in to change notification settings - Fork 187
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: Getting collaborators allows to specify fields #1178
Conversation
String representationHint = "[jpg?dimensions=32x32]"; | ||
ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); | ||
file.getRepresentationContent(representationHint, outputStream); | ||
byte[] downloadedRepresentationContent = outputStream.toByteArray(); | ||
Retry.retry(() -> { |
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.
This fails sometimes so retrying usually solves issue.
* Can be used to override the URL used for file download. | ||
* @return URL for file downalod | ||
*/ | ||
protected URL getDownloadUrl() { |
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.
This of our internal use when we want to change download URL
Pull Request Test Coverage Report for Build #3031
💛 - Coveralls |
Pull Request Test Coverage Report for Build #3020
💛 - Coveralls |
@@ -130,8 +130,14 @@ protected static BoxCollaboration.Info create( | |||
* @param api the API connection to use. | |||
* @return a collection of pending collaboration infos. | |||
*/ | |||
public static Collection<Info> getPendingCollaborations(BoxAPIConnection api) { | |||
URL url = PENDING_COLLABORATIONS_URL.build(api.getBaseURL()); |
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.
is PENDING_COLLABORATIONS_URL used now anywhere? Seems not. Constant to be removed
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.
This is public field. Removing it can break someone else code. I'm suggesting leaving it.
@@ -516,6 +524,11 @@ public void getRepresentationContent(String representationHint, String assetPath | |||
String repContentURLString = null; | |||
while (repContentURLString == null) { | |||
repContentURLString = this.pollRepInfo(representation.getInfo().getUrl()); | |||
try { | |||
Thread.sleep(100); |
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.
why this sleep ?
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.
This is basically infinite loop that is doing as many requests as it can. Only thing that can slow it down is rate limiter. This seems like a least we could improve.
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.
pls address comments
// values = new ArrayList<>(); | ||
// values.add("two"); | ||
// values.add("one"); | ||
// actualMD.add("/otherMultiSelect", values); |
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.
Please don't commit commented code
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 have two options - uncomment it and let it fail or leave for now. Check comment on line 193
. I'm talking with service owners what is going on and I'm going to fix this failing test.
Fix #1177