Skip to content

Commit

Permalink
move access of option out of config and into connect
Browse files Browse the repository at this point in the history
  • Loading branch information
mattseddon committed Mar 3, 2023
1 parent 0f69739 commit e0a8220
Show file tree
Hide file tree
Showing 9 changed files with 56 additions and 64 deletions.
36 changes: 10 additions & 26 deletions extension/src/cli/dvc/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,15 @@ export class DvcRunner extends Disposable implements ICli {
private readonly pseudoTerminal: PseudoTerminal
private currentProcess: Process | undefined
private readonly config: Config
private readonly getStudioAccessToken: () => Thenable<string | undefined>
private readonly getStudioLiveShareToken: () => string | undefined

constructor(
config: Config,
getStudioAccessToken: () => Thenable<string | undefined>
getStudioLiveShareToken: () => string | undefined
) {
super()

this.config = config
this.getStudioAccessToken = getStudioAccessToken
this.getStudioLiveShareToken = getStudioLiveShareToken

this.processCompleted = this.dispose.track(new EventEmitter<CliResult>())
this.onDidCompleteProcess = this.processCompleted.event
Expand Down Expand Up @@ -146,16 +145,8 @@ export class DvcRunner extends Disposable implements ICli {
return this.currentProcess
}

private createProcess({
cwd,
args,
studioAccessToken
}: {
cwd: string
args: Args
studioAccessToken: string | undefined
}): Process {
const options = this.getOptions(cwd, args, studioAccessToken)
private createProcess({ cwd, args }: { cwd: string; args: Args }): Process {
const options = this.getOptions(cwd, args)
const command = getCommandString(options)
const stopWatch = new StopWatch()
const process = this.dispose.track(createProcess(options))
Expand Down Expand Up @@ -183,18 +174,16 @@ export class DvcRunner extends Disposable implements ICli {
return process
}

private getOptions(
cwd: string,
args: Args,
studioAccessToken: string | undefined
) {
private getOptions(cwd: string, args: Args) {
const options = getOptions(
this.config.getPythonBinPath(),
this.config.getCliPath(),
cwd,
...args
)

const studioAccessToken = this.getStudioLiveShareToken()

if (!studioAccessToken) {
return options
}
Expand All @@ -205,18 +194,13 @@ export class DvcRunner extends Disposable implements ICli {
}
}

private async startProcess(cwd: string, args: Args) {
private startProcess(cwd: string, args: Args) {
this.pseudoTerminal.setBlocked(true)
this.processOutput.fire(`Running: dvc ${args.join(' ')}\r\n\n`)

const studioAccessToken = this.config.sendLiveToStudio()
? await this.getStudioAccessToken()
: undefined

this.currentProcess = this.createProcess({
args,
cwd,
studioAccessToken
cwd
})
}
}
7 changes: 0 additions & 7 deletions extension/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,6 @@ export class Config extends DeferredDisposable {
return isPythonExtensionInstalled()
}

public sendLiveToStudio() {
return getConfigValue<boolean>(
ConfigKey.STUDIO_SHARE_EXPERIMENTS_LIVE,
false
)
}

private async getConfigOrExtensionPythonBinPath() {
return getConfigValue(ConfigKey.PYTHON_PATH) || (await getPythonBinPath())
}
Expand Down
28 changes: 23 additions & 5 deletions extension/src/connect/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ import { ContextKey, setContextValue } from '../vscode/context'
import { RegisteredCommands } from '../commands/external'
import { Modal } from '../vscode/modal'
import { GLOBAL_WEBVIEW_DVCROOT } from '../webview/factory'
import { ConfigKey, getConfigValue } from '../vscode/config'

export class Connect extends BaseRepository<undefined> {
public readonly viewKey = ViewKey.CONNECT

private readonly secrets: SecretStorage
private studioAccessToken: string | undefined = undefined

constructor(context: ExtensionContext, webviewIcon: Resource) {
super(GLOBAL_WEBVIEW_DVCROOT, webviewIcon)
Expand All @@ -31,13 +33,20 @@ export class Connect extends BaseRepository<undefined> {
)
)

void this.setContext().then(() => this.deferred.resolve())
void this.getSecret(STUDIO_ACCESS_TOKEN_KEY).then(
async studioAccessToken => {
this.studioAccessToken = studioAccessToken
await this.setContext()
this.deferred.resolve()
}
)

this.dispose.track(
context.secrets.onDidChange(e => {
context.secrets.onDidChange(async e => {
if (e.key !== STUDIO_ACCESS_TOKEN_KEY) {
return
}
this.studioAccessToken = await this.getSecret(STUDIO_ACCESS_TOKEN_KEY)
return this.setContext()
})
)
Expand All @@ -62,8 +71,17 @@ export class Connect extends BaseRepository<undefined> {
return this.storeSecret(STUDIO_ACCESS_TOKEN_KEY, token)
}

public getStudioLiveShareToken() {
return getConfigValue<boolean>(
ConfigKey.STUDIO_SHARE_EXPERIMENTS_LIVE,
false
)
? this.getStudioAccessToken()
: undefined
}

public getStudioAccessToken() {
return this.getSecret(STUDIO_ACCESS_TOKEN_KEY)
return this.studioAccessToken
}

private handleMessageFromWebview(message: MessageFromWebview) {
Expand All @@ -89,8 +107,8 @@ export class Connect extends BaseRepository<undefined> {
return openUrl(`${STUDIO_URL}/user/_/profile?section=accessToken`)
}

private async setContext() {
const storedToken = await this.getStudioAccessToken()
private setContext() {
const storedToken = this.getStudioAccessToken()
if (isStudioAccessToken(storedToken)) {
if (this.deferred.state === 'resolved') {
void Modal.showInformation(
Expand Down
4 changes: 2 additions & 2 deletions extension/src/experiments/commands/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ export const getShareExperimentAsCommitCommand =

export const getShareExperimentToStudioCommand =
(internalCommands: InternalCommands, connect: Connect) =>
async ({ dvcRoot, id }: { dvcRoot: string; id: string }) => {
const studioAccessToken = await connect.getStudioAccessToken()
({ dvcRoot, id }: { dvcRoot: string; id: string }) => {
const studioAccessToken = connect.getStudioAccessToken()
if (!studioAccessToken) {
return commands.executeCommand(RegisteredCommands.CONNECT_SHOW)
}
Expand Down
2 changes: 1 addition & 1 deletion extension/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export class Extension extends Disposable {
this.dvcExecutor = this.dispose.track(new DvcExecutor(config))
this.dvcReader = this.dispose.track(new DvcReader(config))
this.dvcRunner = this.dispose.track(
new DvcRunner(config, () => this.connect.getStudioAccessToken())
new DvcRunner(config, () => this.connect.getStudioLiveShareToken())
)
this.dvcViewer = this.dispose.track(new DvcViewer(config))

Expand Down
32 changes: 14 additions & 18 deletions extension/src/test/suite/cli/dvc/runner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,12 @@ suite('DVC Runner Test Suite', () => {
it('should only be able to run a single command at a time', async () => {
const mockConfig = {
getCliPath: () => 'sleep',
getPythonBinPath: () => undefined,
sendLiveToStudio: () => false
getPythonBinPath: () => undefined
} as Config
const mockGetStudioAccessToken = () => Promise.resolve(undefined)
const mockGetStudioLiveShareToken = () => undefined

const dvcRunner = disposable.track(
new DvcRunner(mockConfig, mockGetStudioAccessToken)
new DvcRunner(mockConfig, mockGetStudioLiveShareToken)
)

const windowErrorMessageSpy = spy(window, 'showErrorMessage')
Expand All @@ -47,13 +46,12 @@ suite('DVC Runner Test Suite', () => {
it('should be able to stop a started command', async () => {
const mockConfig = {
getCliPath: () => 'sleep',
getPythonBinPath: () => undefined,
sendLiveToStudio: () => false
getPythonBinPath: () => undefined
} as Config
const mockGetStudioAccessToken = () => Promise.resolve(undefined)
const mockGetStudioLiveShareToken = () => undefined

const dvcRunner = disposable.track(
new DvcRunner(mockConfig, mockGetStudioAccessToken)
new DvcRunner(mockConfig, mockGetStudioLiveShareToken)
)
const cwd = __dirname

Expand Down Expand Up @@ -116,13 +114,12 @@ suite('DVC Runner Test Suite', () => {

const mockConfig = {
getCliPath: () => 'echo',
getPythonBinPath: () => undefined,
sendLiveToStudio: () => false
getPythonBinPath: () => undefined
} as Config
const mockGetStudioAccessToken = () => Promise.resolve(undefined)
const mockGetStudioLiveShareToken = () => undefined

const dvcRunner = disposable.track(
new DvcRunner(mockConfig, mockGetStudioAccessToken)
new DvcRunner(mockConfig, mockGetStudioLiveShareToken)
)

const started = onDidStartOrCompleteProcess(dvcRunner.onDidStartProcess)
Expand Down Expand Up @@ -155,17 +152,16 @@ suite('DVC Runner Test Suite', () => {

const mockConfig = {
getCliPath: () => 'echo',
getPythonBinPath: () => undefined,
sendLiveToStudio: () => true
getPythonBinPath: () => undefined
} as Config
const mockGetStudioAccessToken = stub()
const mockGetStudioLiveShareToken = stub()
.onFirstCall()
.resolves(mockStudioAccessToken)
.returns(mockStudioAccessToken)
.onSecondCall()
.resolves(undefined)
.returns(undefined)

const dvcRunner = disposable.track(
new DvcRunner(mockConfig, mockGetStudioAccessToken)
new DvcRunner(mockConfig, mockGetStudioLiveShareToken)
)

await dvcRunner.runExperiment(dvcDemoPath)
Expand Down
5 changes: 4 additions & 1 deletion extension/src/test/suite/connect/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ suite('Connect Test Suite', () => {
const connect = disposable.track(
new Connect(
{
secrets: mockSecretStorage || { get: stub(), onDidChange: stub() }
secrets: mockSecretStorage || {
get: stub().resolves(undefined),
onDidChange: stub()
}
} as unknown as ExtensionContext,
resourceLocator.dvcIcon
)
Expand Down
2 changes: 1 addition & 1 deletion extension/src/test/suite/experiments/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@ suite('Experiments Test Suite', () => {
const tokenAccessed = new Promise(resolve =>
mockGetStudioAccessToken.callsFake(() => {
resolve(undefined)
return Promise.resolve(undefined)
return undefined
})
)

Expand Down
4 changes: 1 addition & 3 deletions extension/src/test/suite/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,7 @@ export const bypassProcessManagerDebounce = (
export const buildInternalCommands = (disposer: Disposer) => {
const config = disposer.track(new Config())
const dvcReader = disposer.track(new DvcReader(config))
const dvcRunner = disposer.track(
new DvcRunner(config, () => Promise.resolve(undefined))
)
const dvcRunner = disposer.track(new DvcRunner(config, () => undefined))
const dvcExecutor = disposer.track(new DvcExecutor(config))
const dvcViewer = disposer.track(new DvcViewer(config))
const gitReader = disposer.track(new GitReader())
Expand Down

0 comments on commit e0a8220

Please sign in to comment.