Skip to content

[BlueOyster] Single-domain learning#2231

Merged
cygnusv merged 12 commits intonucypher:blue-oyster-mushroomsfrom
cygnusv:domains
Sep 16, 2020
Merged

[BlueOyster] Single-domain learning#2231
cygnusv merged 12 commits intonucypher:blue-oyster-mushroomsfrom
cygnusv:domains

Conversation

@cygnusv
Copy link
Member

@cygnusv cygnusv commented Sep 8, 2020

As agreed, for the moment we're done with multiple domains. This PR makes the necessary changes to use a single domain everywhere.

Closes:

@cygnusv cygnusv force-pushed the blue-oyster-mushrooms branch from e8338e9 to 21179e3 Compare September 8, 2020 16:50
@cygnusv cygnusv force-pushed the blue-oyster-mushrooms branch from 21179e3 to 7f39e90 Compare September 14, 2020 11:35
@cygnusv cygnusv force-pushed the domains branch 2 times, most recently from 616ad8d to 16241cd Compare September 14, 2020 11:43
@cygnusv cygnusv changed the title [WIP][BlueOyster] Single-domain learning [BlueOyster] Single-domain learning Sep 14, 2020
@cygnusv cygnusv marked this pull request as ready for review September 14, 2020 13:05
"""

VERSION = 1 # bump when static payload scheme changes
VERSION = 2 # bump when static payload scheme changes
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member

@derekpierre derekpierre left a comment

Choose a reason for hiding this comment

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

🎸 - just one suggestion.

@KPrasch KPrasch added BlueOyster🍄 Ursula 👩‍🚀 Effects the "Ursula" development area labels Sep 14, 2020

ALICE = Alice(network_middleware=RestMiddleware(),
domains={TEMPORARY_DOMAIN},
domain=TEMPORARY_DOMAIN,
Copy link
Member

Choose a reason for hiding this comment

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

Ahhh - so refreshing.

self.registry = registry
if domains: # StakeHolder config inherits from character config, which has 'domains' - #1580
self.network = list(domains)[0]
if domain: # StakeHolder config inherits from character config, which has 'domains' - See #1580
Copy link
Member

Choose a reason for hiding this comment

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

Just curious - why is domain optional here?

@@ -1209,7 +1210,7 @@ def run(self,
# if learning: # TODO: Include learning startup here with the rest of the services?
Copy link
Member

Choose a reason for hiding this comment

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

@jMyles - May I draw your attention here 👆

Copy link
Contributor

Choose a reason for hiding this comment

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

So drawn. How can I help?

Copy link
Member

Choose a reason for hiding this comment

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

The TODO says it all

Copy link
Member

@KPrasch KPrasch left a comment

Choose a reason for hiding this comment

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

A comment and a question but no RFCs from me. Looks good!

self.timestamp_bytes(),
bytes(self._interface_signature),
bytes(decentralized_identity_evidence),
bytes(VariableLengthBytestring(self.decentralized_identity_evidence)), # FIXME: Fixed length doesn't work with federated
Copy link
Contributor

Choose a reason for hiding this comment

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

...why? This isn't fixed length?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is but not for federated, where it's a constant sorrow constant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ursula 👩‍🚀 Effects the "Ursula" development area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants