Skip to content
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

Discover Features Protocol: v1_0 refactoring and v2_0 implementation #1500

Merged
merged 12 commits into from
Nov 23, 2021

Conversation

shaangill025
Copy link
Contributor

@shaangill025 shaangill025 commented Nov 17, 2021

Major Changes

  • Startup arguments
    • --auto-disclose-features Enables proactive disclosure for v2.0. Such agent will automatically disclose features to another agent when an active connection gets established. --disclose-features-list can be used to limit what to disclose and if not specified, then all are disclosed.
    • --disclose-features-list Provides control over what features to publish. Accepts a path to YAML config file [example structure below]. This is valid for both v1.0 and v2.0
  • Removed /features under server endpoint
  • Added these endpoints:
    • discover-features tag
      • /discover-features/query
      • /discover-features/records
    • discover-features v2.0 tag
      • /discover-features-2.0/queries
      • /discover-features-2.0/records
  • Added Controller class to protocols to specify, manage and load goal-codes
    YAML config file structure, such agent will only disclose did:sov:BzCbsNYhMrjHiqZDTUASHg;spec/action-menu/1.0 protocol and/or aries.vc goal-code
   protocols: ["did:sov:BzCbsNYhMrjHiqZDTUASHg;spec/action-menu/1.0"]
   goal-codes: ["aries.vc"]

Testing

  • For v1.0, execute query using /discover-features/query. If no connection_id is specified then it processes the query on the same agent (disclose features of the same agent). This essentially mimics what can be achieved using /features [under server]. To look up either all records or a record by connection_id, use /discover-features/records.
  • For v2.0, execute queries using /discover-features-2.0/queries. Same logic when no connection_id is specified as above. To look up either all records or a record by connection_id, use /discover-features-2.0/records.
    connection_id is an unique identifier for these records.

@shaangill025 shaangill025 marked this pull request as ready for review November 19, 2021 17:31
Signed-off-by: Shaanjot Gill <[email protected]>
Signed-off-by: Shaanjot Gill <[email protected]>
Signed-off-by: Shaanjot Gill <[email protected]>
Signed-off-by: Shaanjot Gill <[email protected]>
Signed-off-by: Shaanjot Gill <[email protected]>
@shaangill025 shaangill025 requested a review from ianco November 21, 2021 07:43
Signed-off-by: Shaanjot Gill <[email protected]>
Copy link
Contributor

@ianco ianco 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 small question, but overall looks good to me.


def determine_goal_codes(self) -> Sequence[str]:
"""Return defined goal_codes."""
return []
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this return goal_codes_matching_query()?

Copy link
Contributor Author

@shaangill025 shaangill025 Nov 23, 2021

Choose a reason for hiding this comment

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

In the context of GoalCodeRepository, I am using controller to play a similar role to message_types (with ProtocolsRegistry), determine_goal_code is returning the specified goal-codes for that protocol to build up the registry and then goal_codes_matching_query is called upon in there. I don't think we should be returning goal_codes_matching_query inside determine_goal_codes.

May be, I can rename determine_goal_codes to return_goal_codes to avoid any confusion?

Copy link
Contributor

Choose a reason for hiding this comment

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

@shaangill025 it's fine you don't need to change it, PR is approved :-)

@ianco ianco merged commit e07ed07 into openwallet-foundation:main Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add v2.0 of the Discover Features Protocol - AIP 2.0
2 participants