Codecov Report

Merging #2360 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2360   +/-   ##
=======================================
  Coverage   81.44%   81.44%           
=======================================
  Files          40       40           
  Lines        2323     2323           
=======================================
  Hits         1892     1892           
  Misses        431      431

@muawiakh @shahbazn looks like documentation builds started to fail after the base Docker image was updated (wget was excluded). Should we fix the image versions that we are using to avoid such incidents?

I am not entirely sure when it was removed, also I don't entirely understand by what you mean by fix image versions that we are using? Installing wget inside the docker image would be the ideal scenario IMO, not in the setup.py

oh, if that's the case I can change it

what you mean by fix image versions that we are using

I mean, in the Dockerfile put FROM <os>:<paricular-version>.

@muawiakh

Installing wget inside the docker image would be the ideal scenario IMO, not in the setup.py

Agreed

We have the wget requirement in here
In all docs folders we have different requirements, which are also in setup.py except for the wget. So to me it makes sense to just add it to the setup.py instead of Dockerfile

looks like documentation builds started to fail after the base Docker image was updated (wget was excluded)

Actually, the image is fixed to python:3.6 already. The docs could have been broken when we updated it from 3.5.

I am fine with the current fix. @muawiakh @shahbazn ?

@ldmberman possible that wget installed as part of the image by default before i am not sure how it got broken. will merge this now