Rafał Gaweł (Gerrit)

unread,

9:10 AM (4 hours ago) 9:10 AM

to chromium...@chromium.org, Code Review Nudger, Tomasz Jakubowski, Jacek Klimkowicz, infra-rev...@chromium.org

Attention needed from Jacek Klimkowicz and Tomasz Jakubowski

Rafał Gaweł added 5 comments

File go/src/infra/cros/satlab/common/dut/keystore.go

Line 20, Patchset 1:

Tomasz Jakubowski . unresolved

`defer f.Close()`

Rafał Gaweł

It is already handled inside ReadFile

func ReadFile(name string) ([]byte, error) {
	f, err := Open(name)
	if err != nil {
		return nil, err
	}
	defer f.Close()
	return readFileContents(f)
}

Line 35, Patchset 1:

	data, err := GetKeyStore()
	if err != nil {
		return err
	}

Tomasz Jakubowski . resolved

I think this should be removed. `SetDutWifiInfo` shouldn't fail when the file doesn't exist or when it's corrupted.

Rafał Gaweł

Done

Line 40, Patchset 1:

	data.SSID = ssid
	data.Password = password

Tomasz Jakubowski . resolved

`data := &KeyStore{SSID: ssid, Password: password}`

Rafał Gaweł

Done

File go/src/infra/cros/satlab/common/run/run.go

Line 226, Patchset 1:

			keystore, err := dut.GetKeyStore()
			if err == nil {

Tomasz Jakubowski . resolved

As we use `err` only here, it can be one line.

Rafał Gaweł

Done

File go/src/infra/cros/satlab/common/site/site.go

Line 541, Patchset 1:// KeyStorePath gets the KEystore info file path

Tomasz Jakubowski . resolved

nit: `Keystore`

Rafał Gaweł

Done

Open in Gerrit

Related details

Attention is currently required from:

Jacek Klimkowicz
Tomasz Jakubowski

Submit Requirements:

Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Tomasz Jakubowski (Gerrit)

unread,

9:35 AM (3 hours ago) 9:35 AM

to Rafał Gaweł, chromium...@chromium.org, Code Review Nudger, Jacek Klimkowicz, infra-rev...@chromium.org

Attention needed from Jacek Klimkowicz and Rafał Gaweł

Tomasz Jakubowski voted and added 1 comment

Votes added by Tomasz Jakubowski

Code-Review

+1

1 comment

File go/src/infra/cros/satlab/common/dut/keystore.go

Line 20, Patchset 1:

Tomasz Jakubowski . resolved

`defer f.Close()`

Rafał Gaweł

It is already handled inside ReadFile

func ReadFile(name string) ([]byte, error) {
	f, err := Open(name)
	if err != nil {
		return nil, err
	}
	defer f.Close()
	return readFileContents(f)
}

Tomasz Jakubowski

Acknowledged

Open in Gerrit

Related details

Attention is currently required from:

Jacek Klimkowicz
Rafał Gaweł

Submit Requirements:

Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Rafał Gaweł (Gerrit)

unread,

9:51 AM (3 hours ago) 9:51 AM

to Tomasz Jakubowski, chromium...@chromium.org, Code Review Nudger, Jacek Klimkowicz, infra-rev...@chromium.org

Attention needed from Jacek Klimkowicz

Rafał Gaweł voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention is currently required from:

Jacek Klimkowicz

Submit Requirements:

Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

LUCI CQ (Gerrit)

unread,

10:09 AM (3 hours ago) 10:09 AM

to Rafał Gaweł, Tomasz Jakubowski, chromium...@chromium.org, Code Review Nudger, Jacek Klimkowicz, infra-rev...@chromium.org

LUCI CQ submitted the change

Change information

Commit message:

Setting up Wifi for internal users
Refactor credential storage to make it more generic

BUG=b:460210707
TEST=manual

Change-Id: I34e3731c0c2ebde65b3890ee21447afaf3e52466

Reviewed-on: https://chromium-review.googlesource.com/c/infra/infra/+/7527485

Commit-Queue: Rafał Gaweł <rafal...@google.com>

Reviewed-by: Tomasz Jakubowski <jakub...@google.com>

Cr-Commit-Position: refs/heads/main@{#78814}

Files:

A go/src/infra/cros/satlab/common/dut/keystore.go
A go/src/infra/cros/satlab/common/dut/keystore_test.go
M go/src/infra/cros/satlab/common/run/run.go
M go/src/infra/cros/satlab/common/site/site.go
M go/src/infra/cros/satlab/satlabrpcserver/services/rpc_services/manage_devices_services.go

Change size: M

Delta: 5 files changed, 200 insertions(+), 32 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Tomasz Jakubowski

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

Setting up Wifi for internal users Refactor credential storage to make it more generic [infra/infra : main]

Rafał Gaweł (Gerrit)

Rafał Gaweł added 5 comments

Related details

Tomasz Jakubowski (Gerrit)

Tomasz Jakubowski voted and added 1 comment

Votes added by Tomasz Jakubowski

1 comment

Related details

Rafał Gaweł (Gerrit)

Rafał Gaweł voted Commit-Queue+2

Related details

LUCI CQ (Gerrit)

LUCI CQ submitted the change

Change information