Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

NC-1564: Only subscription owner can unsubscribe #36

Closed
wants to merge 2 commits into from
Closed

NC-1564: Only subscription owner can unsubscribe #36

wants to merge 2 commits into from

Conversation

lucassaldanha
Copy link
Contributor

Only the connections that created a subscription can unsubscribe.

If the connection doesn't own the subscription, we throw a not found exception to avoid
exposing information from others (e.g. scanning existing subscription ids)
*/
if (connectionSubscriptionsMap.get(connectionId) == null
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll suggest moving this logic into a private method, with appropriate name and deleting the comment.

private boolean isExposeSubscriptionDetailsAttempt

}

/*
If the connection doesn't own the subscription, we throw a not found exception to avoid
Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds like behaviour that needs to be documented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've sent a message to team Pliny. I think they haven't worked on the Pub/Sub documentation yet.

shemnon added a commit to shemnon/pantheon that referenced this pull request Mar 12, 2019
Improve the import log line to include relevant metrics like number
of transacitons, ommers, and gas usage.
Also, this fixes a bug in that we get task times when metrics are not
turned on.

Sample line:
2019-03-11 21:03:50.245+00:00 | EthScheduler-Workers-3 | INFO  | BlockPropagationManager | Imported PegaSysEng#36 / 20 tx / 0 om / 420,000 (4.0%) gas / (0xecb366469d88d9d75b20721581797c1eb1118557b93db9a460dc0e000b9ec7fe) in 0.143s.
shemnon added a commit to shemnon/pantheon that referenced this pull request Mar 12, 2019
Improve the import log line to include relevant metrics like number
of transacitons, ommers, and gas usage.
Also, this fixes a bug in that we get task times when metrics are not
turned on.

Sample line:
2019-03-11 21:03:50.245+00:00 | EthScheduler-Workers-3 | INFO  | BlockPropagationManager | Imported PegaSysEng#36 / 20 tx / 0 om / 420,000 (4.0%) gas / (0xecb366469d88d9d75b20721581797c1eb1118557b93db9a460dc0e000b9ec7fe) in 0.143s.
shemnon added a commit to shemnon/pantheon that referenced this pull request Mar 12, 2019
Improve the import log line to include relevant metrics like number
of transacitons, ommers, and gas usage.
Also, this fixes a bug in that we get task times when metrics are not
turned on.

Sample line:
2019-03-11 21:03:50.245+00:00 | EthScheduler-Workers-3 | INFO  | BlockPropagationManager | Imported PegaSysEng#36 / 20 tx / 0 om / 420,000 (4.0%) gas / (0xecb366469d88d9d75b20721581797c1eb1118557b93db9a460dc0e000b9ec7fe) in 0.143s.
shemnon added a commit that referenced this pull request Mar 12, 2019
Improve the import log line to include relevant metrics like number
of transactions, ommers, and gas usage.
Also, this fixes a bug in that we get task times when metrics are not
turned on.

Sample line:
2019-03-11 21:03:50.245+00:00 | EthScheduler-Workers-3 | INFO  | BlockPropagationManager | Imported #36 / 20 tx / 0 om / 420,000 (4.0%) gas / (0xecb366469d88d9d75b20721581797c1eb1118557b93db9a460dc0e000b9ec7fe) in 0.143s.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants