docs(*:skip) Update SuperNode docker example #3722
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Robert-Steiner
force-pushed
the
doc/update-supernode-docker-image
branch
7 times, most recently
from
9ccd11f
to
dbb94bb
Compare
chongshenng
requested changes
Jul 18, 2024
There was a problem hiding this comment.
Hi @Robert-Steiner, I've reviewed mainly the Quickstart and have suggested some changes. About including dependencies in the Dockerfile, I prefer if we can show how to use pyproject.toml to reduce copy-pasting dependencies across files. Can you also check the flwr run execution? I think it's missing some arguments to run on the Docker SuperExec/SuperLink.
Robert-Steiner
force-pushed
the
doc/update-supernode-docker-image
branch
from
4a0bab1
to
cae146e
Compare
Robert-Steiner
force-pushed
the
doc/update-supernode-docker-image
branch
2 times, most recently
from
8e383e9
to
47e3867
Compare
Signed-off-by: Robert Steiner <[email protected]>
Signed-off-by: Robert Steiner <[email protected]>
Signed-off-by: Robert Steiner <[email protected]>
Signed-off-by: Robert Steiner <[email protected]>
Signed-off-by: Robert Steiner <[email protected]>
Signed-off-by: Robert Steiner <[email protected]>
Signed-off-by: Robert Steiner <[email protected]>
Signed-off-by: Robert Steiner <[email protected]>
Signed-off-by: Robert Steiner <[email protected]>
Signed-off-by: Robert Steiner <[email protected]>
Signed-off-by: Robert Steiner <[email protected]>
Signed-off-by: Robert Steiner <[email protected]>
Signed-off-by: Robert Steiner <[email protected]>
Signed-off-by: Robert Steiner <[email protected]>
Signed-off-by: Robert Steiner <[email protected]>
Robert-Steiner
force-pushed
the
doc/update-supernode-docker-image
branch
2 times, most recently
from
46fde7e
to
bd7032e
Compare
Signed-off-by: Robert Steiner <[email protected]>
Robert-Steiner
force-pushed
the
doc/update-supernode-docker-image
branch
from
bd7032e
to
ccf511b
Compare
chongshenng
requested changes
Jul 25, 2024
There was a problem hiding this comment.
Thanks @Robert-Steiner for the updates! A few more minor suggestions on the other pages.
|
|
||
| $ docker run --rm \ | ||
| --volume ./ca.crt:/app/ca.crt/:ro \ | ||
| flwr/supernode:|current_flwr_version| \ |
There was a problem hiding this comment.
Suggested change
| flwr/supernode:|current_flwr_version| \ | |
| flwr/supernode:|stable_flwr_version| \ |
| | current working directory of the host machine as a read-only volume at the ``/app/ca.crt`` | ||
| | directory inside the container. | ||
| * | :substitution-code:`flwr/supernode:|current_flwr_version|`: The name of the image to be run and the specific | ||
| | tag of the image. The tag :substitution-code:`|current_flwr_version|` represents a specific version of the image. |
There was a problem hiding this comment.
Suggested change
| | tag of the image. The tag :substitution-code:`|current_flwr_version|` represents a specific version of the image. | |
| | tag of the image. The tag :substitution-code:`|stable_flwr_version|` represents a specific version of the image. |
|
|
||
| $ docker run --rm \ | ||
| --volume ./certificates/:/app/certificates/:ro \ | ||
| flwr/superexec:|current_flwr_version| \ |
There was a problem hiding this comment.
Suggested change
| flwr/superexec:|current_flwr_version| \ | |
| flwr/superexec:|stable_flwr_version| \ |
| | This allows the container to access the TLS certificates that are stored in the certificates | ||
| | directory. | ||
| * | :substitution-code:`flwr/superexec:|current_flwr_version|`: The name of the image to be run and the specific | ||
| | tag of the image. The tag :substitution-code:`|current_flwr_version|` represents a specific version of the image. |
There was a problem hiding this comment.
Suggested change
| | tag of the image. The tag :substitution-code:`|current_flwr_version|` represents a specific version of the image. | |
| | tag of the image. The tag :substitution-code:`|stable_flwr_version|` represents a specific version of the image. |
Co-authored-by: Chong Shen Ng <[email protected]>
Robert-Steiner
force-pushed
the
doc/update-supernode-docker-image
branch
from
da06b05
to
157f15a
Compare
Signed-off-by: Robert Steiner <[email protected]>
Robert-Steiner
force-pushed
the
doc/update-supernode-docker-image
branch
from
157f15a
to
9135310
Compare
@chongshenng, thanks for the great feedback! I tried to address all the comments. Let me know in case I missed one. Regarding renaming |
Signed-off-by: Robert Steiner <[email protected]>
Robert-Steiner
force-pushed
the
doc/update-supernode-docker-image
branch
from
e636b51
to
e318719
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue
Description
Related issues/PRs
Proposal
Explanation
Checklist
#contributions)Any other comments?