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

Refer to jfrog container instead of docker hub (Daml Shell image) #773

Merged
merged 2 commits into from
Oct 18, 2024

Conversation

iggy-da
Copy link
Collaborator

@iggy-da iggy-da commented Aug 1, 2024

No description provided.

@iggy-da iggy-da requested review from a team as code owners August 1, 2024 03:42
Copy link
Contributor

@carrielaben-da carrielaben-da left a comment

Choose a reason for hiding this comment

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

We should provide a link for the reader to find the relevant release notes but other than that this is good to go.

Copy link
Contributor

@dylant-da dylant-da left a comment

Choose a reason for hiding this comment

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

Minor changes, aside from that looks good.

@@ -50,11 +50,11 @@ Running Daml Shell
==================

You can run Daml Shell from a ``jar`` artifact or from a ``Docker``
image. See the Release Notes for download instructions.
image. See the Release Notes for download instructions.
Copy link
Contributor

Choose a reason for hiding this comment

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

Like Carrie, I'm not sure what release notes refers to here, a link would be useful - I don't see any download instructions on the Github releases if that's where it's supposed to link to?


.. code-block:: sh

docker run -it --net host docker.io/digitalasset/daml-shell:<version-tag> # change --net host if desired
docker run -it --net host digitalasset-docker.jfrog.io/daml-shell:<version-tag> # change --net host if desired
Copy link
Contributor

Choose a reason for hiding this comment

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

An example URL for a specific tag would be useful - it's not obvious where the user should type daml-shell:v0.1.4 or daml-shell:0.1.4 here.


.. code-block:: sh

docker run -it --net host docker.io/digitalasset/daml-shell:<version-tag> # change --net host if desired
docker run -it --net host digitalasset-docker.jfrog.io/daml-shell:<version-tag> # change --net host if desired
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't test the command here actually works - my docker credentials + my artifactory credentials are presumably not sufficient. It would likely be good to add a sub-section on what an authentication failure looks like and how to remedy it, since the error I get:

docker: Error response from daemon: Head "https://digitalasset-docker.jfrog.io/v2/daml-shell/manifests/v0.1.4": unknown: Authentication is required.

is not terribly informative

@iggy-da iggy-da enabled auto-merge (squash) October 18, 2024 07:42
@iggy-da iggy-da merged commit ee04c1d into main Oct 18, 2024
1 check passed
@iggy-da iggy-da deleted the 2024-08-01-daml-shell branch October 18, 2024 08:29
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.

4 participants