Improving restoration of store nodes metadata#2265
Merged
cygnusv merged 10 commits intonucypher:blue-oyster-mushroomsfrom Sep 22, 2020
Merged
Improving restoration of store nodes metadata#2265cygnusv merged 10 commits intonucypher:blue-oyster-mushroomsfrom
cygnusv merged 10 commits intonucypher:blue-oyster-mushroomsfrom
Conversation
jMyles
reviewed
Sep 19, 2020
7fba47c to
0fa885a
Compare
jMyles
approved these changes
Sep 20, 2020
fjarri
reviewed
Sep 20, 2020
fjarri
reviewed
Sep 20, 2020
fjarri
reviewed
Sep 20, 2020
derekpierre
reviewed
Sep 21, 2020
0fa885a to
cf8913f
Compare
Member
|
Needs rebase over |
KPrasch
reviewed
Sep 21, 2020
| uris.extend(hardcoded_uris) | ||
|
|
||
| return uris | ||
| # TODO: This module seems unused |
Member
There was a problem hiding this comment.
🤔 - This was originally intended to be a dehydration method for CLI and API actions, like loading up a collection of stranger-Ursulas before having instantiated yourself. I recall discussing with @jMyles recently about this. Perhaps it's been integrated via TI?
KPrasch
reviewed
Sep 21, 2020
| # | ||
| # | ||
| # | ||
| # def aggregate_seednode_uris(domains: set, highest_priority: Optional[List[str]] = None) -> List[str]: |
Member
There was a problem hiding this comment.
This function in particular is very cooluseful, IMO. I'd like to preserve the functionality instead of deprecate it.
KPrasch
reviewed
Sep 21, 2020
| # uris = highest_priority or list() | ||
| # for domain in domains: | ||
| # | ||
| # # 2 - Static nodes from JSON file |
KPrasch
reviewed
Sep 21, 2020
tests/mock/performance_mocks.py
Outdated
| if self._actual_rest_app is None: | ||
| self._actual_rest_app, _datastore = make_rest_app(db_filepath=tempfile.mkdtemp(), | ||
| this_node=self.this_node, | ||
| serving_domains=(None,)) |
Member
There was a problem hiding this comment.
Thanks goodness this was cleaned up. 😆
…rent domain Demonstrates nucypher#1843
… nodes from other domains
cf8913f to
395135f
Compare
Co-authored-by: Derek Pierre <derek.pierre@gmail.com>
derekpierre
approved these changes
Sep 22, 2020
|
|
||
| # Metadata | ||
| self.__temp_metadata_dir = tempfile.mkdtemp(prefix="nucypher-tmp-nodes-") | ||
| self.__temp_metadata_dir = str(Path(self.__temp_root_dir) / "metadata") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR demonstrates that currently it's possible to learn about nodes from a different domain when they are recovered from storage, which can happen for example when a staker reuses the configuration from a testnet. It then fixes it by introducing a simple domain check when restoring nodes from disk, ignoring nodes that don't belong to current domain.
Closes #1843
Closes #1623