Skip to content
This repository was archived by the owner on May 4, 2024. It is now read-only.

Conversation

@ea-open-source
Copy link
Contributor

@ea-open-source ea-open-source commented Jun 27, 2022

Motivation

The move movey-upload command from #227 requires users to save their Movey's API token locally, which is inconvenient and error-prone to do manually.
This PR adds move movey-login command, which allows users to save Movey's API Token using the CLI.

How to use it

Step 1: Run the following command on the terminal:
move movey-login
Step 2: Enter your Movey API Token (you can get it on Movey), and the token will be saved in $HOME/.move/movey_credential.toml.

Have you read the Contributing Guidelines on pull requests?

Yes.

Test Plan

cargo test --package move-cli --test cli_tests save_credential
cargo test --package move-cli --lib save_credential

@ea-open-source
Copy link
Contributor Author

tnowacki, wrwg, awelc, damirka
Would you guys help us with rerunning Actions and reviews?

@ea-open-source ea-open-source changed the title Add command to save Movey API Token [Movey] Add command to save Movey API Token Jul 18, 2022
@ea-open-source ea-open-source force-pushed the features/login-command branch from e9724ec to 9414e8a Compare August 2, 2022 11:19
@awelc awelc self-assigned this Aug 3, 2022
@@ -0,0 +1,228 @@
// Copyright (c) The Diem Core Contributors
Copy link
Collaborator

Choose a reason for hiding this comment

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

A nit. For new files, use only the "Move contributors" part of the copyright notice.

[dependencies]
anyhow = "1.0.52"
difference = "2.0.0"
dirs-next = "2.0.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit. Is this a better (newer?) version of the dirs crate (https://crates.io/crates/dirs)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We chose version 2.0.0 because it already existed in another place.
We can update both later.


pub fn save_credential(token: String, move_home: String) -> Result<()> {
fs::create_dir_all(&move_home)?;
let credential_path = move_home + MOVEY_CREDENTIAL_PATH;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Have you tested this on Windows? I have a feeling that this style of concatenating paths will not work there. It's used in other places too (e.g., in tests) - please check that all this works on Windows.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We've tested both #226 and #227 and can confirm that they works.

use std::{fs, fs::File, io, path::PathBuf};
use toml_edit::easy::{map::Map, Value};

pub const MOVEY_CREDENTIAL_PATH: &str = "/movey_credential.toml";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also see comment below - I doubt this path format will work on windows.

}
let mut toml: Value = old_contents
.parse()
.map_err(|e| anyhow::Error::from(e).context("could not parse input as TOML"))?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would probably be useful to include file path in error message so that the user can act on it. For similar reason, you may add something like "Delete file to regenerate it with default settings".

@awelc awelc merged commit de01244 into move-language:main Aug 8, 2022
awelc pushed a commit that referenced this pull request Aug 12, 2022
* upload metadata to movey

# Conflicts:
#	Cargo.lock
#	language/tools/move-cli/Cargo.toml

* Add api token to upload request, get token from credential file

# Conflicts:
#	Cargo.lock
#	language/tools/move-cli/tests/cli_tests.rs

* Show error message if upload with bad credentials

# Conflicts:
#	Cargo.lock

* Refactor move_cli's utils & add cli tests

# Conflicts:
#	language/tools/move-cli/tests/cli_tests.rs

* Remove `description, total_size` field from request body, change movey staging url

* call movey api after getting dependency

# Conflicts:
#	language/tools/move-package/Cargo.toml
#	language/tools/move-package/src/resolution/resolution_graph.rs

* Change dev movey_url, send params in post request

# Conflicts:
#	language/tools/move-package/src/resolution/resolution_graph.rs

* Refactor error message when upload failed, add reqwest crate

* Get long rev, use cli_exe path from env

* Refactor test code to run in parallel

* Refactor tests to run in current dir, flatten `upload` command

* Refactor to be specific to Movey, change UploadRequest's params

- MoveyUploadRequest: +subdir, -rev
- Remove integration test of head commit id

* Refactor MOVE_HOME env, MOVEY_URL to common package

* Refactor test logic in uploading, remove `increase download` logic

* Add Movey server Mock, refactor test code in upload

- Add more integration test to Movey upload
- Remove unused stub move packages
- Upgrade `log` crate version
- Add `httpmock` crate

* Rename move_command_line_common::movey.rs to movey_constants.rs

* Change movey api url, refactor getting credential key, refactor tests

* Rename movey api, change the message of successful upload request

* Refactor and add comments for movey tests

* Refactor and rebase according to #226 changes

* Refactor mock movey server function, remove unnecessary license

- change `init_server_mock` to `mock_movey_server_with_response_body_and_status_code` to be more inline with the comment
- remove `// Copyright (c) The Diem Core Contributors`

* Move `MOVEY_CREDENTIAL_PATH` to common package, refactor logic of reading credential file
brson pushed a commit to brson/move that referenced this pull request Jul 11, 2023
…e#226)

This patch extends the baseline `move-native` `vec_cmp_eq` routine
to handle vector<vector<T>>.

Uncomment `test_vec_vec_bool` in the stdlib-bcs serializer test
(which now passes).

Add more tests to vec.move to exercise vec_cmp_eq with vec-of-vec.
brson pushed a commit to brson/move that referenced this pull request Jul 17, 2023
…e#226)

This patch extends the baseline `move-native` `vec_cmp_eq` routine
to handle vector<vector<T>>.

Uncomment `test_vec_vec_bool` in the stdlib-bcs serializer test
(which now passes).

Add more tests to vec.move to exercise vec_cmp_eq with vec-of-vec.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants