Skip to content
This repository has been archived by the owner on Mar 11, 2024. It is now read-only.

Allow container source image to be configured #4

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jwerle
Copy link

@jwerle jwerle commented Jan 14, 2020

No description provided.

@jwerle jwerle requested a review from mikeseese January 14, 2020 18:04
@mikeseese
Copy link
Contributor

Hi @jwerle, what's the use case for this change? Can you please provide more details? I'm not seeing a reason jumping out to support merging

@jwerle
Copy link
Author

jwerle commented Jan 15, 2020

One use case for us is allowing us to specify options like the server where an image lives. The work around for now is to use the client.request() method manually

Copy link
Contributor

@mikeseese mikeseese left a comment

Choose a reason for hiding this comment

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

In addition to the other comment/suggestion, the typing of image needs to be updated in the function definition of both createContainer and launchContainer. See https://github.com/trufflesuite/ts-lxd/pull/4/files#diff-d554f9108d9df7d1caac4ea44d302504R231 and https://github.com/trufflesuite/ts-lxd/pull/4/files#diff-d554f9108d9df7d1caac4ea44d302504R235

This type should be something like image: string | IImageDefinition where IImageDefinition is an interface I'd like for you to define and it should reflect the available options at https://github.com/lxc/lxd/blob/master/doc/rest-api.md#post-optional-targetmember. It can be placed near the top around line 32.

src/Client.ts Outdated Show resolved Hide resolved
Co-Authored-By: Mike Seese <[email protected]>
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