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

New external service to check stats documentation #80

Closed
mpadge opened this issue Jul 12, 2022 · 9 comments
Closed

New external service to check stats documentation #80

mpadge opened this issue Jul 12, 2022 · 9 comments

Comments

@mpadge
Copy link
Collaborator

mpadge commented Jul 12, 2022

@xuanxu @maelle I'm pretty sure you can both ignore this one. Implementation should just work.

@noamross Idea here is to implement a command for pre-submission stats enquiries, @ropensci-review-bot check srr, which will report on standards compliance to report total number of standards complied with, and confirm that that is > 50%. Any thoughts or comments?

@mpadge
Copy link
Collaborator Author

mpadge commented Jul 12, 2022

Actually @xuanxu two questions:

  1. We don't want buffy to send any message here. Is that done with message: -> empty body?
  2. We also don't want buffy to deliver any response from the external service; how do we ensure that? The service itself will dump the response (which may take some minutes).

In other words, we don't want buffy to dump anything into the issue here at all, just to pass on parameters to the external service. How to we best ensure that?

@xuanxu
Copy link
Collaborator

xuanxu commented Jul 13, 2022

  • We don't want buffy to send any message here. Is that done with message: -> empty body?

No need for an empty body, if there is no message: param nothing will be sent

  • We also don't want buffy to deliver any response from the external service; how do we ensure that? The service itself will dump the response (which may take some minutes).

For this you can use the silent param: silent: true will do

mpadge added a commit that referenced this issue Jul 13, 2022
@mpadge
Copy link
Collaborator Author

mpadge commented Jul 15, 2022

Thanks @xuanxu, i think i've got everything in place now, including the external service spec via the above PR, but the command was not recognised. Any ideas why not?

@xuanxu
Copy link
Collaborator

xuanxu commented Jul 15, 2022

Is the command listed if you run @ropensci-review-bot help ?

@mpadge
Copy link
Collaborator Author

mpadge commented Jul 15, 2022

Nope

@xuanxu
Copy link
Collaborator

xuanxu commented Jul 15, 2022

I saw the problem: the external_service is repetead in the config file, defined multiple times. All external services should be declared as an array under the same external_service entry

@mpadge
Copy link
Collaborator Author

mpadge commented Aug 5, 2022

@xuanxu (I've been away on holidays, but now back and still trying to get this to work ..) The new config file with the external service matrix is this:

external_service:
- editorcheck:
only:
- editors
authorized_roles_in_issue:
- author1
- author-others
- reviewers-list
command: check package
description: Various package checks
message: Thanks, about to send the query.
url: http://138.68.123.59:8000/editorcheck
method: get
query_params:
secret: <%= ENV['PKGCHECK_TOKEN'] %>
data_from_issue:
- repo
- repourl
- issue_id
template_file: goodpractice.md
- srrcheck:
only:
- editors
command: check srr
description: Checks srr documentation for stats packages
url: http://138.68.123.59:8000/srrcheck
method: get
query_params:
secret: <%= ENV['PKGCHECK_TOKEN'] %>
data_from_issue:
- repo
- repourl
- issue_id
silent: true

The requests as sent to the external service then look like this:

GET /editorcheck ?data_from_issue%5B%5D=repo&data_from_issue%5B%5D=repourl&data_from_issue%5B%5D=issue_id&secret 200 0.002
GET /srrcheck ?data_from_issue%5B%5D=repo&data_from_issue%5B%5D=repourl&data_from_issue%5B%5D=issue_id&secret 200 0.002

so it seems buffy is sending an empty data_from_issue parameter instead of actually parsing the array for the variables and values. Does that seem right to you?


@maelle This is the reason for responses like these that just dump

Error: Issue template has no 'repourl'

because the parameters are no longer being passed correctly to the external service. I think this issue underlies all behaviour noted in ropensci-review-tools/roreviewapi#30

@xuanxu
Copy link
Collaborator

xuanxu commented Aug 9, 2022

👋 @mpadge I've open a PR to fix the problem: #83

@maelle
Copy link
Collaborator

maelle commented Aug 22, 2022

Can this be closed or are there remaining problems?

@mpadge mpadge closed this as completed Aug 22, 2022
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

No branches or pull requests

3 participants