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

Address dependabot alerts, regenerate constraints.txt #1250

Merged
merged 4 commits into from May 12, 2022

Conversation

SpacemanPaul
Copy link
Contributor

Reason for this pull request

Addresses upstream security issues identified by Dependabot.

Proposed changes

  • Update docs/rtd requirements

  • Update constraints.in and regenerated constraints.txt

  • Closes #xxxx

  • Tests added / passed

  • Fully documented, including docs/about/whats_new.rst for all changes

@codecov
Copy link

codecov bot commented May 11, 2022

Codecov Report

Merging #1250 (d68bf51) into develop (049b7c1) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop    #1250   +/-   ##
========================================
  Coverage    93.62%   93.62%           
========================================
  Files          129      129           
  Lines        13134    13135    +1     
========================================
+ Hits         12297    12298    +1     
  Misses         837      837           
Impacted Files Coverage Δ
datacube/scripts/search_tool.py 97.29% <100.00%> (+0.03%) ⬆️

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 049b7c1...d68bf51. Read the comment docs.

@@ -11,7 +11,7 @@ click>=8.0
cloudpickle>=0.4
compliance-checker>=4.0.0
dask>=2021.5.1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dask and distributed versions should be the same ideally

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I suspect that will be easier to figure out than the click version issue the tests have thrown up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SpacemanPaul
Copy link
Contributor Author

Ugh - looks like there might be further issues arising from a new version of pip released overnight.

@SpacemanPaul SpacemanPaul marked this pull request as ready for review May 12, 2022 05:09
@SpacemanPaul SpacemanPaul merged commit 93cfa48 into develop May 12, 2022
@SpacemanPaul SpacemanPaul deleted the dask_distrib_security_fix branch May 12, 2022 05:32
Copy link
Member

@omad omad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Paul, looks good.

Can you remove the mentions of paramiko completely before merging.

@@ -9,9 +9,9 @@ celery>=4,<5
ciso8601
click>=8.0
cloudpickle>=0.4
compliance-checker>=4.0.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, compliance checker is only for checking NetCDF metadata compliance.

If we ever get to the point of making NetCDF support optional, it can be optional too.

@@ -1,4 +1,4 @@
paramiko==2.7.1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

paramiko can go entirely, it's not used anywhere at all.

@@ -35,7 +35,7 @@ netCDF4==1.5.3
numpy==1.22.2
packaging==20.3
pandas==1.0.3
paramiko==2.7.1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above, remove completely, not required or even used.

@@ -27,7 +28,7 @@ def printable_values(d):
return {k: printable(v) for k, v in d.items()}


def write_pretty(out_f, field_names, search_results, terminal_size=click.get_terminal_size()):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh my, I don't even think this datacube-search thing should exist any more. I don't know why it ever did. The one or two commands it supports should be moved under datacube dataset or datacube product`.

@@ -9,6 +9,7 @@

import csv
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does anyone else dislike the naming of this module scripts. IMO it should be cli.

Maybe it doesn't matter, but I think of scripts as being quite different to a fully fledged and essential command line tool...

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

Successfully merging this pull request may close these issues.

None yet

3 participants