-
Notifications
You must be signed in to change notification settings - Fork 78
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
Fix update docs workflow step failure #359
Conversation
Focusing on this part of the failure:
|
Setting this in the docs requirement.txt was not enough:
|
I think pandas didnt' update dependencies for 3.10.0 until 1.3.4 |
I think I'm a little confused on how this works. In order to use a different pandas version it would have to be published to pypi not just change the fidesctl requirements file. Side note: Doesn't that mean this job should depend on the publish package job? |
It's not clear why this started happening but I think something changed between ubuntu versions where this old error is now being propagated and failing the job. If you look at older jobs this error is not new: I think rather than trying to figure that out we can just address the error we've been seeing:
This is due to using python 3.10 numpy/numpy#19033:
The dependency on NumPy 1.19.3 comes from pandas 1.3.0. Sooo we have two options:
I think to update pandas we would also have to publish a new version of fidesctl to pypi so for now I'm okay just setting the python version. |
What do you think of the two options above? I am partially thinking that we should push a new build of fidesctl to pypi with a newer pandas version. It is possible to use fidesctl 1.3.1 with python 3.10 and our hardcoded version of pandas(1.3.0) is not compatible with python 3.10. I don't know why this isnt causing more issues but I'm pretty sure that's why our docs python build is failing. |
@earmenda let's both pin the python version and bump the pandas version in this PR. we can remove the python version pin in a later PR and this will also set up the fix for 1.4.0 release |
@ThomasLaPiana sounds good to me, updated. Added a comment to remove the explicit version later on also |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving, feel free to merge when tests pass. thanks for this fix!
Closes #358
Code Changes
Steps to Confirm
Pre-Merge Checklist
Description Of Changes
Will use this PR to trigger a failure and then confirm a fix. Commenting out the deploy part of the action:
Run failure: https://github.com/ethyca/fides/runs/5150462980