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

Add command for managing key value stores and links #162

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

kate-goldenring
Copy link
Collaborator

DO NOT MERGE until changes are merged in fermyon/cloud-openapi#11

Aims to reuse as much logic and keep error formatting consistent via the link_utils module.

Copy link
Contributor

@rylev rylev left a comment

Choose a reason for hiding this comment

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

Looking great!

src/commands/key_value.rs Outdated Show resolved Hide resolved
src/commands/key_value.rs Outdated Show resolved Hide resolved
.get_key_value_stores(None)
.await
.with_context(|| format!("Error listing key value stores '{}'", self.name))?;
let found = list.iter().find(|kv| kv.name == self.name);
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to eventually have an endpoint to find a store by name so that we don't have to fetch all stores?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree. Lets add this in a later iteration, though

src/commands/key_value.rs Outdated Show resolved Hide resolved
r#"Database "{}" is now linked to app "{}" with the label "{}""#,
self.database, self.app, self.label
.map(|s| ResourceLinks::new(s.name, s.links))
.collect::<Vec<_>>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: you can avoid collecting this into a Vec if you change the link function to take an impl Iterator<Item = ResourceLink>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If i pass an iterator to link the first search of any will consume part of the iterator corrupting it for other analysis later in the function

Copy link
Contributor

Choose a reason for hiding this comment

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

You can use by_ref on Iterator to avoid this.

src/commands/link_utils.rs Outdated Show resolved Hide resolved
@kate-goldenring kate-goldenring merged commit f53b8ad into fermyon:main Jan 18, 2024
8 checks passed
@kate-goldenring kate-goldenring deleted the kv-links branch January 18, 2024 23:12
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