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

docs: dfx extension cli & design doc #3433

Closed
wants to merge 10 commits into from
Closed

Conversation

smallstepman
Copy link
Contributor

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.

Fixes # (issue)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.

Checklist:

  • The title of this PR complies with Conventional Commits.
  • I have edited the CHANGELOG accordingly.
  • I have made corresponding changes to the documentation.

@smallstepman smallstepman requested review from chenyan-dfinity and a team as code owners November 3, 2023 15:43
@smallstepman smallstepman force-pushed the mnl/extension-docs branch 3 times, most recently from e64f575 to 477c8ad Compare November 3, 2023 15:47
docs/design/dfx-extensions.md Outdated Show resolved Hide resolved

2. Download and Extraction:
- Once the compatible version of the extension is determined, dfx constructs a download URL. This URL points to a GitHub releases page where the extensions are hosted. Currently, extensions support downloading only from `dfinity/dfx-extensions` repository. Here is how the URL template looks like:
```
Copy link
Member

Choose a reason for hiding this comment

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

The three backticks should not be indented. This seems to mess up the formatting going forward.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted the codeblock to indented together with bullet point, I made the adjustment, what do you think https://github.com/dfinity/sdk/blob/caefa3588ac04446a0c818a9c431d7da2a129673/docs/design/dfx-extensions.md

docs/design/dfx-extensions.md Outdated Show resolved Hide resolved
docs/design/dfx-extensions.md Outdated Show resolved Hide resolved
docs/design/dfx-extensions.md Outdated Show resolved Hide resolved
docs/design/dfx-extensions.md Outdated Show resolved Hide resolved
- If the extension name is not associated with the dfx version, another error is raised.
- The versions of the extension that are compatible with the dfx version are then sorted, and the latest version is chosen as the most suitable one.
- This mechanism ensures that users always get the newest version of an extension that works correctly with their dfx version.
- In essence, while dfx provides tools to manage extensions, the compatibility matrix and extension manifest add an extra layer of sophistication. They ensure that extensions are accurately described, can be transformed into command-line interfaces, and most importantly, are always in sync with the version of dfx the user is operating. This comprehensive management system guarantees smooth operations and enhances user experience.
Copy link
Member

Choose a reason for hiding this comment

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

Reads more like a sales pitch than a technical design.

Does the document need to describe an "extra layer of sophistication" or a "comprehensive management system" that "guarantees smooth operations and enhances user experience." ?

1. Determine Extension Compatibility:
- Before any installation can take place, dfx checks if the requested extension is already installed. If it is, the process terminates with an error.
- The utility identifies the version of the extension compatible with its own version. This ensures that users don't end up installing extensions that may not work properly with their specific version of dfx.
- Compatibility is determined using an extension compatibility matrix, which presumably maps extensions to compatible versions of dfx.
Copy link
Member

@ericswanson-dfinity ericswanson-dfinity Nov 4, 2023

Choose a reason for hiding this comment

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

Overall, there's either too much detail here, or it's not organized in a way that it can be consumed. I would expect a reader's response to seeing this document be "TLDR". As a starting point, 13 bullet points for one topic is too many.


When requested to run an extension, dfx fetches the location of the executable associated with the extension, by determining the cache location associated with its current version, and searching for directory with extension name inside: `$(dfx cache show)/extensions/EXT_NAME/EXT_NAME`. Fox example , when you're running `dfx sns ARGS` or `dfx extension run sns ARGS`, `dfx` will try to find the extension binary at this path: `$(dfx cache show)/extensions/sns/sns`.

When executing the binary, the path to the cache is appended as an argument when invoking the extension's executable, in practice that means the extension will always be executed with `--dfx-cache-path $(dfx cache show)` parameter, like so: `exec $(dfx cache show)/extensions/sns/sns --dfx-cache-path $(dfx cache show)`. This allows the extension to depend on other extensions, and binaries stored in dfx's cache.
Copy link
Member

Choose a reason for hiding this comment

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

Rather than prose, it would be better to describe which parameters dfx passes to the extension binary in the form of a table or list. A prose description / example can follow.

Otherwise, buried in the middle of the paragraph is the fact that dfx passes --dfx-cache-path "$(dfx cache show)".

- On UNIX systems, appropriate permissions are set for the installed extension to ensure it's executable.

4. Error Handling:
- Throughout this process, any potential errors are diligently checked for, ensuring a robust installation process. Examples include errors related to incompatible versions, download failures, decompression errors, etc.
Copy link
Member

Choose a reason for hiding this comment

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

More back-patting: diligently and robust. Checking for errors like download failures and decompression errors should be a given and do not need to be mentioned in this document. This reads like filler, maybe written by chatgpt.

Comment on lines +34 to +36
- Post-extraction, the extension is renamed and moved to its permanent location (the directory where extensions are meant to reside).
- If the user specifies a custom name for the extension at the time of installation, the binary is renamed to reflect this custom name.
- On UNIX systems, appropriate permissions are set for the installed extension to ensure it's executable.
Copy link
Member

Choose a reason for hiding this comment

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

There's so much noise here that the important pieces will be easy to miss.

Suggested change
- Post-extraction, the extension is renamed and moved to its permanent location (the directory where extensions are meant to reside).
- If the user specifies a custom name for the extension at the time of installation, the binary is renamed to reflect this custom name.
- On UNIX systems, appropriate permissions are set for the installed extension to ensure it's executable.
- If the user specifies a custom name for the extension at the time of installation, the binary is renamed to reflect this custom name.


## dfx extension run

**Running an Extension**:
Copy link
Member

@ericswanson-dfinity ericswanson-dfinity Nov 4, 2023

Choose a reason for hiding this comment

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

Should we call out the fact that dfx extension run is not the recommended way to call an extension? tbh I'm don't really understand why extension run exists at all as a valid syntax.


For example, below commands will execute `sns` extension:
```bash
dfx sns
Copy link
Member

Choose a reason for hiding this comment

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

This should be a command that actually does something.

Suggested change
dfx sns
dfx sns install

# DFX extensions - how it works

`dfx` extension is a feature in the DFINITY Developer Software Development Kit (SDK) that allows for extending the `dfx` CLI's core functionality. Modeled after the principles of [Git custom commands](https://mfranc.com/tools/git-custom-command/) and [Cargo's custom subcommands](https://doc.rust-lang.org/book/ch14-05-extending-cargo.html#extending-cargo-with-custom-commands), the feature enables the addition of user-defined commands that seamlessly integrate with the existing `dfx` command set.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe start by thinking about what are the things that you think a reader should understand after reading this document, and organize the document around that.

Comment on lines +30 to +31
- The extension is then downloaded from this URL. If the download fails for any reason, an error is returned.
- After successful download, the extension, which is in a compressed archive format (.tar.gz), is unpacked to a temporary directory.
Copy link
Member

@ericswanson-dfinity ericswanson-dfinity Nov 4, 2023

Choose a reason for hiding this comment

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

extraneous

Suggested change
- The extension is then downloaded from this URL. If the download fails for any reason, an error is returned.
- After successful download, the extension, which is in a compressed archive format (.tar.gz), is unpacked to a temporary directory.


## dfx extension install

**Installing an Extension**:
Copy link
Member

Choose a reason for hiding this comment

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

Please describe the --install-as and --version optional arguments here.

@@ -0,0 +1,83 @@
# dfx extension
Copy link
Member

Choose a reason for hiding this comment

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

Please add a link to this file from the index

dfx extension install sns
```

This command will install the `sns` extension.
Copy link
Member

Choose a reason for hiding this comment

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

Please describe what happens if the extension is already installed. (note: I have filed https://dfinity.atlassian.net/browse/SDK-1309 to make extension install behavior consistent with things like apt-get install and brew install).


Use the `dfx extension` command to manage the extensions available in the `dfx` tool. Extensions can provide additional functionality and commands to the `dfx` tool, enhancing its capabilities.

## Basic Usage
Copy link
Member

Choose a reason for hiding this comment

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

Somewhere, please describe how to upgrade to a newer version of an extension.

@ericswanson-dfinity ericswanson-dfinity deleted the mnl/extension-docs branch May 6, 2024 21:59
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.

2 participants