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

Walkthrough update and deployment fixes #258

Merged
merged 2 commits into from
Aug 27, 2024
Merged

Walkthrough update and deployment fixes #258

merged 2 commits into from
Aug 27, 2024

Conversation

setrofim
Copy link
Collaborator

Fix deployments to use the latest clients (and some param passing issues in docker deployment), and update end-to-end/walkthrough.md to work with the latest code line (plus various clarity and usability enhancements).

This addresses #256

Copy link
Contributor

@thomas-fossati thomas-fossati left a comment

Choose a reason for hiding this comment

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

Ah, the joys of quoting :-) Awesome, thanks!

(I've inlined a couple of editorial suggestions)

deployments/docker/README.md Outdated Show resolved Hide resolved
deployments/docker/README.md Outdated Show resolved Hide resolved
deployments/docker/README.md Outdated Show resolved Hide resolved
deployments/docker/README.md Show resolved Hide resolved
- Fix quoting in docker deployment parameter passing. This removes the
  need for weird double-quoting around media types, and is generally
  more robust and consistent.
- Various other robustness/shellcheck fixes in docker deployment.
- Fix api_server URL inside docker deployment cocli config to use HTTPS
- Add config  file for evcli inside the docker deployment, removing the
  need to specify --api-server on invocation. As well as general
  convenience, this allows for docker and native deployment commands to
  be invoked with identical parameters, removing the need to
  differentiate between the two.
- Update evcli and (in case of native) cocli to use the latest version.
  These contain evcli config fixes, and the support for the latest CoRIM
  format.

Signed-off-by: Sergei Trofimov <[email protected]>
@setrofim setrofim force-pushed the doc-fix branch 2 times, most recently from 56b0f9c to cd10a32 Compare August 27, 2024 10:03
Update the end-to-end walkthrough to reflect the latest changes to the
code line, and to improve its maintainability going forward.

- Mention native deployment as an alternative to Docker.
- Remove the installation instructions, pointing to the deployments'
  READMEs instead, and only giving the actual make command.
- Remove references to external `cocli` and `evcli` tools, instead
  relying on the ones that are part of the deployment and on the JSON
  "templates" that are part of this code base. This way, there is no
  risk of things getting out of sync.
- Remove relative paths from commands, instead using pre-defined root
  locations. This removes ambiguity as to where things are (are paths
  relative to the file, to the repo, PWD, etc), and make commands more
  robust when being executed via copying and pasting, as they won't rely
  on PWD.
- Adjust the instructions so that no new files are created or existing
  files modified in the source-controlled repo. Instead use a dedicated
  working directory created as part of the walkthrough for any new or
  modified files. This makes it clearer afterwards what was
  changed/crated, and reduced the possibility of stray changes being
  committed.
- Update CoRIM/CoMID "template" listings to reflect the updated formats.
- Reformat for readability and consistency with the rest of
  documentation: 79 column lines, use ```sh for shell listings.
- Minor typo fixes.

In addition to the walkthrough itself, the following related updates are
included:

- Update end-to-end inputs build script to build evidence, as well as
  corims from source.
- Update deployments' READMEs to include "git clone" followed by "cd" as
  part of the initial listing to give better context to where the rest
  of the commands in the READMEs are supposed to be executed.

Signed-off-by: Sergei Trofimov <[email protected]>
Copy link
Contributor

@thomas-fossati thomas-fossati left a comment

Choose a reason for hiding this comment

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

Thanks!

> modify the template to use `memory` backend for the stores).
> [!NOTE]
> The individual services rely on configuration inside `config.yaml.template`.
> This currently uses the `sqlite3` driver for the store backend, which limits
Copy link
Collaborator

@yogeshbdeshpande yogeshbdeshpande Aug 27, 2024

Choose a reason for hiding this comment

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

Should we not have the options for other drivers at least in the documentation, I mean Postgres and other driver added recently?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not for the Docker deployment, as Postgres/MariaDB do not currently run in the deployment. In the future, we may wish to switch the deployment to use one of those, rather than sqlite3; but that's a bigger change.

Copy link
Collaborator

@yogeshbdeshpande yogeshbdeshpande left a comment

Choose a reason for hiding this comment

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

LGTM!

@setrofim setrofim merged commit 4f50718 into main Aug 27, 2024
9 checks passed
@setrofim setrofim deleted the doc-fix branch August 27, 2024 11:02
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.

3 participants