Skip to content

Commit

Permalink
fix AccountName refs and remove DeployedAt
Browse files Browse the repository at this point in the history
  • Loading branch information
mmarchetti committed Nov 9, 2023
1 parent e5868ae commit c17ee6b
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 68 deletions.
48 changes: 22 additions & 26 deletions cmd/connect-client/commands/publish_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,11 @@ package commands
import (
"encoding/json"
"testing"
"time"

"github.com/rstudio/connect-client/internal/accounts"
"github.com/rstudio/connect-client/internal/cli_types"
"github.com/rstudio/connect-client/internal/logging"
"github.com/rstudio/connect-client/internal/state"
"github.com/rstudio/connect-client/internal/types"
"github.com/rstudio/connect-client/internal/util"
"github.com/rstudio/connect-client/internal/util/utiltest"
"github.com/spf13/afero"
Expand All @@ -26,19 +24,19 @@ func TestPublishCommandSuite(t *testing.T) {
suite.Run(t, new(PublishCommandSuite))
}

func (s *PublishCommandSuite) createSavedState(path util.Path, accountName, configName string) {
func (s *PublishCommandSuite) createSavedState(path util.Path, accountName, configName string) *state.Deployment {
// This fixture simulates executing a publish command,
// which will create a saved state directory.
deployment := state.NewDeployment()
deployment.Target.AccountName = accountName

cmd := &PublishCmd{
BaseBundleCmd: &BaseBundleCmd{
PublishArgs: cli_types.PublishArgs{
Path: path,
State: deployment,
Config: configName,
New: false,
Path: path,
AccountName: accountName,
Config: configName,
New: false,
State: deployment,
},
},
}
Expand All @@ -52,10 +50,9 @@ func (s *PublishCommandSuite) createSavedState(path util.Path, accountName, conf
err := cmd.LoadState(ctx)
s.NoError(err)

cmd.State.Target.DeployedAt = types.NewOptional(time.Now())

err = cmd.SaveState(ctx)
s.NoError(err)
return cmd.State
}

func (s *PublishCommandSuite) assertStateExists(path util.Path, name string) {
Expand All @@ -73,7 +70,6 @@ func (s *PublishCommandSuite) assertStateExists(path util.Path, name string) {
decoder := json.NewDecoder(f)
decoder.DisallowUnknownFields()
decoder.Decode(&target)
s.Equal("test", target.AccountName)
}

func (s *PublishCommandSuite) TestSaveState() {
Expand Down Expand Up @@ -111,23 +107,23 @@ func (s *PublishCommandSuite) TestLoadStateDefaultAccountNoPriorDeployments() {
}
err := cmd.LoadState(ctx)
s.NoError(err)
s.Equal("test", cmd.State.Target.AccountName)
s.Equal("test", cmd.AccountName)
}

func (s *PublishCommandSuite) TestLoadStateWithAccountNoPriorDeployments() {
// Account name is provided on the CLI, but there are no prior deployments.
deployment := state.NewDeployment()
deployment.Target.AccountName = "test"
afs := afero.NewMemMapFs()
path := util.NewPath("/", afs)

cmd := &PublishCmd{
BaseBundleCmd: &BaseBundleCmd{
PublishArgs: cli_types.PublishArgs{
Path: path,
State: deployment,
Config: "",
New: false,
Path: path,
AccountName: "test",
Config: "",
New: false,
State: deployment,
},
},
}
Expand All @@ -139,7 +135,7 @@ func (s *PublishCommandSuite) TestLoadStateWithAccountNoPriorDeployments() {
}
err := cmd.LoadState(ctx)
s.NoError(err)
s.Equal("test", cmd.State.Target.AccountName)
s.Equal("test", cmd.AccountName)
}

func (s *PublishCommandSuite) TestLoadStateWithAccountAndPriorDeployments() {
Expand All @@ -150,15 +146,15 @@ func (s *PublishCommandSuite) TestLoadStateWithAccountAndPriorDeployments() {
s.createSavedState(path, "not-it", "")

deployment := state.NewDeployment()
deployment.Target.AccountName = "test"

cmd := &PublishCmd{
BaseBundleCmd: &BaseBundleCmd{
PublishArgs: cli_types.PublishArgs{
Path: path,
State: deployment,
Config: "",
New: false,
Path: path,
AccountName: "test",
Config: "",
New: false,
State: deployment,
},
},
}
Expand All @@ -170,7 +166,7 @@ func (s *PublishCommandSuite) TestLoadStateWithAccountAndPriorDeployments() {
}
err := cmd.LoadState(ctx)
s.NoError(err)
s.Equal("test", cmd.State.Target.AccountName)
s.Equal("test", cmd.AccountName)
}

func (s *PublishCommandSuite) TestLoadStateNoAccountAndPriorDeployments() {
Expand All @@ -179,7 +175,7 @@ func (s *PublishCommandSuite) TestLoadStateNoAccountAndPriorDeployments() {
afs := afero.NewMemMapFs()
path := util.NewPath("/", afs)
s.createSavedState(path, "older", "")
s.createSavedState(path, "newer", "")
newState := s.createSavedState(path, "newer", "")

deployment := state.NewDeployment()

Expand All @@ -201,7 +197,7 @@ func (s *PublishCommandSuite) TestLoadStateNoAccountAndPriorDeployments() {
}
err := cmd.LoadState(ctx)
s.NoError(err)
s.Equal("newer", cmd.State.Target.AccountName)
s.Equal(newState, cmd.State)
}

func (s *PublishCommandSuite) TestGetDefaultAccountEmpty() {
Expand Down
28 changes: 16 additions & 12 deletions internal/bundles/manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,6 @@ func (s *ManifestSuite) TestNewManifest() {
s.Equal(1, manifest.Version)
s.Empty(manifest.Packages)
s.Empty(manifest.Files)
s.Nil(manifest.Python)
s.Nil(manifest.Jupyter)
s.Nil(manifest.Quarto)
s.Nil(manifest.Environment)
}

func (s *ManifestSuite) TestAddFile() {
Expand All @@ -62,10 +58,14 @@ func (s *ManifestSuite) TestReadManifest() {
manifest, err := ReadManifest(reader)
s.Nil(err)
s.Equal(&Manifest{
Version: 1,
Platform: "4.1.0",
Packages: PackageMap{},
Files: ManifestFileMap{},
Version: 1,
Platform: "4.1.0",
Python: &Python{},
Quarto: &Quarto{},
Jupyter: &Jupyter{},
Environment: &Environment{},
Packages: PackageMap{},
Files: ManifestFileMap{},
}, manifest)
}

Expand Down Expand Up @@ -100,10 +100,14 @@ func (s *ManifestSuite) TestReadManifestFile() {
manifest, err := ReadManifestFile(manifestPath)
s.Nil(err)
s.Equal(&Manifest{
Version: 1,
Platform: "4.1.0",
Packages: PackageMap{},
Files: ManifestFileMap{},
Version: 1,
Platform: "4.1.0",
Python: &Python{},
Quarto: &Quarto{},
Jupyter: &Jupyter{},
Environment: &Environment{},
Packages: PackageMap{},
Files: ManifestFileMap{},
}, manifest)
}

Expand Down
1 change: 0 additions & 1 deletion internal/publish/publish.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,6 @@ func (p *Publisher) publishWithClient(
ContentName: "",
Username: account.AccountName,
BundleId: types.NewOptional(bundleID),
DeployedAt: types.NewOptional(time.Now()),
}

taskID, err := withLog(events.PublishDeployBundleOp, "Initiating bundle deployment", "task_id", log, func() (types.TaskID, error) {
Expand Down
2 changes: 0 additions & 2 deletions internal/services/api/deployments/services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ func (s *ServicesSuite) TestSetDeploymentTitle() {

func (s *ServicesSuite) TestSetDeploymentAccount() {
src := state.NewDeployment()
s.Equal(src.Target.AccountName, "")
s.Equal(src.Target.ServerType, accounts.ServerType(""))
s.Equal(src.Target.ServerURL, "")

Expand All @@ -79,7 +78,6 @@ func (s *ServicesSuite) TestSetDeploymentAccount() {
res, err := service.SetDeploymentAccount(lister, "test")
s.Nil(err)

s.Equal(res.Target.AccountName, result.Name)
s.Equal(res.Target.ServerType, result.ServerType)
s.Equal(res.Target.ServerURL, result.URL)

Expand Down
16 changes: 3 additions & 13 deletions internal/state/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,8 @@ type TargetID struct {
ContentName types.ContentName `json:"content_name" help:"Name of content item to update."` // Content Name (unique per user)

// These fields are informational and don't affect future deployments.
Username string `json:"username,omitempty"` // Username, if known
BundleId types.NullBundleID `json:"bundle_id"` // Bundle ID that was deployed
DeployedAt types.NullTime `json:"deployed_at"` // Date/time bundle was deployed
Username string `json:"username,omitempty"` // Username, if known
BundleId types.NullBundleID `json:"bundle_id"` // Bundle ID that was deployed
}

type LocalDeploymentID string
Expand Down Expand Up @@ -197,16 +196,7 @@ func listDeployments(sourceDir util.Path, log logging.Logger) ([]*Deployment, er
}
}
sort.Slice(deployments, func(i, j int) bool {
// Sort in reverse order by deployment time
t1, t1valid := deployments[i].Target.DeployedAt.Get()
t2, t2valid := deployments[j].Target.DeployedAt.Get()
if t1valid && t2valid {
return t1.After(t2)
} else if t1valid {
return true
} else {
return false
}
return deployments[i].Target.ServerURL < deployments[j].Target.ServerURL
})
return deployments, nil
}
Expand Down
28 changes: 16 additions & 12 deletions internal/state/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ func (s *DeploymentSuite) TestMergeEmpty() {
orig := NewDeployment()
orig.SourceDir = util.NewPath("/my/dir", nil)
orig.PythonRequirements = []byte("numpy\npandas\n")
orig.Target.AccountName = "my-account"
orig.Target.ServerType = accounts.ServerTypeConnect
orig.Target.ServerURL = "https://connect.example.com"
orig.Target.ContentId = "abc123"
Expand All @@ -51,7 +50,6 @@ func (s *DeploymentSuite) TestMergeNonEmpty() {
orig := NewDeployment()
orig.SourceDir = util.NewPath("/my/dir", nil)
orig.PythonRequirements = []byte("numpy\npandas\n")
orig.Target.AccountName = "my-account"
orig.Target.ServerType = accounts.ServerTypeConnect
orig.Target.ServerURL = "https://connect.example.com"
orig.Target.ContentId = "abc123"
Expand All @@ -61,15 +59,13 @@ func (s *DeploymentSuite) TestMergeNonEmpty() {
other := NewDeployment()
other.SourceDir = util.NewPath("/other/dir", nil)
other.PythonRequirements = []byte("flask\n")
other.Target.AccountName = "your-account"
other.Target.ServerType = accounts.ServerTypeShinyappsIO
other.Target.ServerURL = "https://shinyapps.io"
other.Target.ContentId = types.ContentID("99")
other.Target.ContentName = types.ContentName("my-app")
merged.Merge(other)
s.Equal(other.SourceDir, merged.SourceDir)
s.Equal([]byte("numpy\npandas\nflask\n"), merged.PythonRequirements)
s.Equal("your-account", merged.Target.AccountName)
s.Equal(accounts.ServerTypeShinyappsIO, merged.Target.ServerType)
s.Equal("https://shinyapps.io", merged.Target.ServerURL)
s.Equal(types.ContentID("99"), merged.Target.ContentId)
Expand All @@ -90,10 +86,14 @@ func (s *DeploymentSuite) TestLoadManifest() {
err = deployment.LoadManifest(path, log)
s.Nil(err)
s.Equal(bundles.Manifest{
Version: 1,
Platform: "4.1.0",
Packages: bundles.PackageMap{},
Files: bundles.ManifestFileMap{},
Version: 1,
Platform: "4.1.0",
Python: &bundles.Python{},
Quarto: &bundles.Quarto{},
Jupyter: &bundles.Jupyter{},
Environment: &bundles.Environment{},
Packages: bundles.PackageMap{},
Files: bundles.ManifestFileMap{},
}, deployment.Manifest)
}

Expand All @@ -111,10 +111,14 @@ func (s *DeploymentSuite) TestLoadManifestDir() {
err = deployment.LoadManifest(path, log)
s.Nil(err)
s.Equal(bundles.Manifest{
Version: 1,
Platform: "4.1.0",
Packages: bundles.PackageMap{},
Files: bundles.ManifestFileMap{},
Version: 1,
Platform: "4.1.0",
Python: &bundles.Python{},
Quarto: &bundles.Quarto{},
Jupyter: &bundles.Jupyter{},
Environment: &bundles.Environment{},
Packages: bundles.PackageMap{},
Files: bundles.ManifestFileMap{},
}, deployment.Manifest)
}

Expand Down
1 change: 0 additions & 1 deletion web/src/api/types/deployments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ export type Target = {
contentName: string;
username: string;
bundleId: string | null;
deployedAt: number | null;
}

export type Deployment = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ describe('description', () => {
contentName: '',
username: '',
bundleId: null,
deployedAt: null
},
manifest: {
version: 1,
Expand Down

0 comments on commit c17ee6b

Please sign in to comment.