#502 kraken resubscribe issue #551
Conversation
| @@ -27,6 +20,14 @@ | |||
| import org.slf4j.Logger; | |||
| import org.slf4j.LoggerFactory; | |||
|
|
|||
| import java.math.BigDecimal; | |||
There was a problem hiding this comment.
Could you run a maven build so the coveo-fmt plugin runs?
We've introduced that to avoid formatting issues like these creeping into PRs.
Thanks!
| private ObjectMapper mapper = StreamingObjectMapperHelper.getObjectMapper(); | ||
| private final boolean isPrivate; | ||
|
|
||
| private final Map<Integer, String> subscriptionRequestMap = new ConcurrentHashMap<>(); | ||
| private final Map<Integer, Set<String>> subscriptionRequestMap = new ConcurrentHashMap<>(); |
There was a problem hiding this comment.
You may find guava synchronized multimaps deal with this better and more thread-safely (not convinced that the remove() is being used in a thread-safe way).
| + KRAKEN_CHANNEL_DELIMITER | ||
| + currencyPair; | ||
| channelsList.remove(channelName); | ||
| if (channelsList.isEmpty()) { |
There was a problem hiding this comment.
This is the bit I'm not convinced is thread-safe.
| subscriptionName + (depth == null ? "" : (KRAKEN_CHANNEL_DELIMITER + depth)), | ||
| d -> | ||
| new KrakenSubscriptionMessage( | ||
| Math.abs(UUID.randomUUID().hashCode()), |
There was a problem hiding this comment.
This will probably collide quite often. Maybe safer to use a static AtomicLong? Does it need to be unique?
| HashMap<String, KrakenSubscriptionMessage> messages = new HashMap<>(); | ||
| for (Map.Entry<String, Subscription> entry : super.channels.entrySet()) { | ||
|
|
||
| String[] channelData = entry.getKey().split(KRAKEN_CHANNEL_DELIMITER); |
There was a problem hiding this comment.
nit: this would be easier to read if maybe you created a KrakenSubscription object and added a decode method, e.g.
private KrakenSubscription decodeSubscription(Subscription subscription) {}
badgerwithagun
added
the
awaiting_fixes
|
This project is in the process of being merged into the XChange project and no further PRs will be merged here. Once the projects have been merged, there may be a short stabilization period where there will be large-scale renaming of classes and packages, which may cause conflicts. You are advised to wait at least a week from now and then resubmit your PR on the XChange project. Thank you for your support! |
|
You can now resubmit your PR on XChange. This project will shortly be marked as archived. |
#502 issue fix
Batch resubscribing has been implemented