You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@fluo.apache.org by GitBox <gi...@apache.org> on 2020/02/02 09:11:52 UTC
[GitHub] [fluo-muchos] arvindshmicrosoft opened a new pull request #317:
Correctly retry ASF mirror listing task
arvindshmicrosoft opened a new pull request #317: Correctly retry ASF mirror listing task
URL: https://github.com/apache/fluo-muchos/pull/317
The previous addition of retry was ineffective because it had a
failed_when condition - which needs to be an 'until' condition to
correctly detect success.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [fluo-muchos] arvindshmicrosoft commented on a change in pull
request #317: Correctly retry ASF mirror listing task
Posted by GitBox <gi...@apache.org>.
arvindshmicrosoft commented on a change in pull request #317: Correctly retry ASF mirror listing task
URL: https://github.com/apache/fluo-muchos/pull/317#discussion_r374290592
##########
File path: ansible/roles/accumulo/tasks/download.yml
##########
@@ -19,10 +19,10 @@
shell: curl -sk https://apache.org/mirrors.cgi?as_json | grep preferred | cut -d \" -f 4
args:
warn: no
+ register: apache_mirror
retries: 10
delay: 10
- register: apache_mirror
- failed_when: "'http' not in apache_mirror.stdout"
+ until: "'http' in apache_mirror.stdout"
Review comment:
Thank you for your inputs, @ctubbsii ! Based on the same I have incorporated jq to parse the JSON and also cleaned up the code to reference a shared task. I also tested and verified that things are work correctly in both happy path and failure path (i.e. the limited retries mechanism are working correctly).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [fluo-muchos] arvindshmicrosoft merged pull request #317: Correctly
retry ASF mirror listing task
Posted by GitBox <gi...@apache.org>.
arvindshmicrosoft merged pull request #317: Correctly retry ASF mirror listing task
URL: https://github.com/apache/fluo-muchos/pull/317
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [fluo-muchos] ctubbsii commented on a change in pull request #317:
Correctly retry ASF mirror listing task
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #317: Correctly retry ASF mirror listing task
URL: https://github.com/apache/fluo-muchos/pull/317#discussion_r373833382
##########
File path: ansible/roles/accumulo/tasks/download.yml
##########
@@ -19,10 +19,10 @@
shell: curl -sk https://apache.org/mirrors.cgi?as_json | grep preferred | cut -d \" -f 4
args:
warn: no
+ register: apache_mirror
retries: 10
delay: 10
- register: apache_mirror
- failed_when: "'http' not in apache_mirror.stdout"
+ until: "'http' in apache_mirror.stdout"
Review comment:
The retrieval of the mirror list is a bit unreliable, since it is depending on the unstable json formatting that the server just happens to return. If we could ensure the machines had `jq` installed, we could easily parse the json instead. Something like:
```yaml
shell: curl -sk https://apache.org/mirrors.cgi?as_json | jq -r .preferred
```
I'm not sure if it's possible to base the retry on the the command return codes, but if it is, you AND the return codes from both commands in the pipe with something like:
```yaml
shell: curl -sk https://apache.org/mirrors.cgi?as_json | jq -r .preferred; [[ ${PIPESTATUS[0]} -eq 0 && ${PIPESTATUS[1]}
```
I don't know YAML/Ansible well enough to know how much bash could work there without getting crazy with the escaping, but if you could retry until the command is successful, that might be better than checking for 'http' in the output... which could also appear in an error message (and also 'preferred' could return ftp).
At the very least, parsing using `jq` instead of `grep` could improve this retrieval / retry code a bit. (Or, if you don't want to install `jq`, you could do the same in python, but it's uglier: `| python3 -c "import sys, json; print(json.load(sys.stdin)['preferred'])"`).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [fluo-muchos] arvindshmicrosoft commented on a change in pull
request #317: Correctly retry ASF mirror listing task
Posted by GitBox <gi...@apache.org>.
arvindshmicrosoft commented on a change in pull request #317: Correctly retry ASF mirror listing task
URL: https://github.com/apache/fluo-muchos/pull/317#discussion_r374290592
##########
File path: ansible/roles/accumulo/tasks/download.yml
##########
@@ -19,10 +19,10 @@
shell: curl -sk https://apache.org/mirrors.cgi?as_json | grep preferred | cut -d \" -f 4
args:
warn: no
+ register: apache_mirror
retries: 10
delay: 10
- register: apache_mirror
- failed_when: "'http' not in apache_mirror.stdout"
+ until: "'http' in apache_mirror.stdout"
Review comment:
Thank you for your inputs, @ctubbsii ! Based on the same I have incorporated jq to parse the JSON and also cleaned up the code to reference a shared task. I also tested and verified that things are work correctly in both happy path and failure path (i.e. the limited retries mechanism are working correctly). If you are good with these changes, kindly resolve the conversation post which I can merge.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services