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

Fix defects from PR#777 #781

Merged
merged 3 commits into from
Oct 28, 2020
Merged

Fix defects from PR#777 #781

merged 3 commits into from
Oct 28, 2020

Conversation

jmgrady
Copy link
Collaborator

@jmgrady jmgrady commented Oct 27, 2020

PR #777 introduced 2 defects:

  • in certmgr/Dockerfile, the destination directory for the entrypoint scripts needs a trailing '/'
  • from the review, the python scripts were updated to use pathlib instead of strings for file paths. One function, Path.readlink() is not available until Python 3.9; the certmgr container is at Python 3.7. The 2 uses of Path.readlink() are changed to use os.readlink()

This change is Reviewable

@jmgrady jmgrady added bug Something isn't working docker labels Oct 27, 2020
@jmgrady jmgrady self-assigned this Oct 27, 2020
@codecov-io
Copy link

codecov-io commented Oct 27, 2020

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #781   +/-   ##
=======================================
  Coverage   51.44%   51.44%           
=======================================
  Files         235      235           
  Lines        6276     6276           
  Branches      404      404           
=======================================
  Hits         3229     3229           
  Misses       2749     2749           
  Partials      298      298           
Flag Coverage Δ
#backend 55.83% <ø> (ø)
#frontend 47.32% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


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 e9ec5d4...02c1bfb. Read the comment docs.

Path.readlink is not available until Python 3.9.
@jmgrady jmgrady changed the title Fix script destination directory Fix defects from PR#777 Oct 27, 2020
@jmgrady jmgrady marked this pull request as ready for review October 27, 2020 15:27
Copy link
Collaborator

@imnasnainaec imnasnainaec left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @jmgrady)

@jmgrady jmgrady merged commit a1544c1 into master Oct 28, 2020
@jmgrady jmgrady deleted the certmgr_dockerfile_fix branch October 28, 2020 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working docker
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants