Skip to content
This repository has been archived by the owner on Mar 27, 2024. It is now read-only.

externalize agent parameters + router #3229

Merged
merged 13 commits into from
Apr 27, 2022

Conversation

cnuss
Copy link
Contributor

@cnuss cnuss commented Apr 25, 2022

Title:
Allow External Usage of agentParameters and mux.Router

Description:
To allow for another server framework than golang's http, we need to externalize the creation of mux.NewRouter().

Summary:
Related: #3224

When @borancar and I first authored #3224, we copied and pasted start.go and named it aires.go to gain access to mux.NewRouter(), so we could plug it into another server framework, as Lambda does not use golang's http package, however it is compatible with gorillamux.

So this PR allows us to import startcmd and leverage a couple new functions:

  • NewAgentParameters()
    • AgentParameters is now an exported type
    • AgentParameters has a new method: NewRouter()
  • nil-safety on helper functions for cobra.Command
    • Our lambda integration will use environment variables exclusively, so we pass nil when constructing AgentParameters

@cnuss
Copy link
Contributor Author

cnuss commented Apr 25, 2022

FYI @troyronda @rolsonquadras @borancar this is released to #3224 to remove the copied/pasted aries.go we added

@cnuss
Copy link
Contributor Author

cnuss commented Apr 25, 2022

@rolsonquadras I fixed some linting issues can you allow the workflows to run?

@cnuss cnuss requested a review from rolsonquadras April 25, 2022 23:11
@cnuss
Copy link
Contributor Author

cnuss commented Apr 26, 2022

@rolsonquadras sorry for the back and fourth, i enabled GH Actions on my Fork to get all the other Unit Test/Linting issues resolved.

We should be good to go now if you can run workflows once again!

fqutishat and others added 7 commits April 26, 2022 09:07
Signed-off-by: Firas Qutishat <firas.qutishat@securekey.com>
Signed-off-by: Christian Nuss <christian.nuss@gmail.com>
Signed-off-by: Christian Nuss <christian.nuss@gmail.com>
Signed-off-by: Christian Nuss <christian.nuss@gmail.com>
The method always returns nil. It's there just to allow it to implement a "Pinger" sort of interface which may be defined somewhere.

Signed-off-by: Derek Trider <Derek.Trider@securekey.com>
Signed-off-by: Christian Nuss <christian.nuss@gmail.com>
Signed-off-by: Christian Nuss <christian.nuss@gmail.com>
Signed-off-by: Christian Nuss <christian.nuss@gmail.com>
Signed-off-by: Christian Nuss <christian.nuss@gmail.com>
@rolsonquadras
Copy link
Contributor

@rolsonquadras sorry for the back and fourth, i enabled GH Actions on my Fork to get all the other Unit Test/Linting issues resolved.

We should be good to go now if you can run workflows once again!

@cnuss - tagging @sudeshrshetty @fqutishat @troyronda as they have access to run the workflows

@cnuss cnuss force-pushed the feat/rest_externalize branch from 13d9cd9 to e03e60c Compare April 26, 2022 16:08
@codecov
Copy link

codecov bot commented Apr 26, 2022

Codecov Report

Merging #3229 (f8399f3) into main (2435073) will increase coverage by 0.00%.
The diff coverage is 65.92%.

@@           Coverage Diff           @@
##             main    #3229   +/-   ##
=======================================
  Coverage   88.72%   88.72%           
=======================================
  Files         309      309           
  Lines       41704    41716   +12     
=======================================
+ Hits        37002    37014   +12     
  Misses       3424     3424           
  Partials     1278     1278           
Impacted Files Coverage Δ
cmd/aries-agent-rest/startcmd/start.go 79.25% <65.92%> (+0.47%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2435073...f8399f3. Read the comment docs.

@troyronda
Copy link
Contributor

@cnuss It looks like the branch is out-of-date with the base branch.

@cnuss
Copy link
Contributor Author

cnuss commented Apr 27, 2022

@troyronda thank you, i just sync'd with main!

could you trigger GH Actions again?

also added a unit test to get hopefully get codecov in order 🙏

cc: @sudeshrshetty @fqutishat

cnuss added 2 commits April 27, 2022 08:43
Signed-off-by: Christian Nuss <christian.nuss@gmail.com>
Signed-off-by: Christian Nuss <christian.nuss@gmail.com>
@cnuss
Copy link
Contributor Author

cnuss commented Apr 27, 2022

@troyronda @sudeshrshetty @fqutishat these linting issues, first time collaborator pauses, and codecov breakages are really frustrating, and the only way i can figure out i've done something wrong is by committing then waiting

could one of you you attempt to push this PR over the finish line? i'm now having an unrelated unit test failure, probably because i'm using os.Setenv (appartently i can't use t.Setenv because we're on an older version of go?)

i've given each of you maintainer access to https://github.com/scaffoldly/aries-framework-go

@fqutishat fqutishat merged commit ae41d52 into hyperledger-archives:main Apr 27, 2022
@troyronda
Copy link
Contributor

@cnuss This PR has been merged. Thanks for your contribution!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants