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

cli: set project on cfg #2526

Merged
merged 5 commits into from
Oct 20, 2021
Merged

cli: set project on cfg #2526

merged 5 commits into from
Oct 20, 2021

Conversation

krantzinator
Copy link
Contributor

Fixes #2523

Also closes #2525; Izaak and I chatted in Slack about preferring this strategy.

@krantzinator krantzinator added pr/no-changelog No automatic changelog entry required for this pull request backport/0.6.x labels Oct 20, 2021
@krantzinator krantzinator requested a review from a team October 20, 2021 16:21
@github-actions github-actions bot added the core label Oct 20, 2021
@krantzinator krantzinator removed the pr/no-changelog No automatic changelog entry required for this pull request label Oct 20, 2021
@krantzinator krantzinator added this to the 0.6.x milestone Oct 20, 2021
@izaaklauer
Copy link
Contributor

I wonder if there are any commands that check if that project ref is nil, and do some project-agnostic thing if so - maybe status? I'll test.

Copy link
Member

@briancain briancain left a comment

Choose a reason for hiding this comment

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

Gave this a shot with waypoint status, waypoint logs, and waypoint exec and it works as expected!

@izaaklauer
Copy link
Contributor

izaaklauer commented Oct 20, 2021

I think that this block will never trigger now:

if baseCfg.ProjectTargetRequired {
if c.refProject == nil {
if len(c.cfg.Project) == 0 {
c.ui.Output(errProjectMode, terminal.WithErrorStyle())
return ErrSentinel
}
c.refProject = &pb.Ref_Project{
Project: c.cfg.Project,
}
}
}

@krantzinator
Copy link
Contributor Author

I think that this block will never trigger now:

if baseCfg.ProjectTargetRequired {
if c.refProject == nil {
if len(c.cfg.Project) == 0 {
c.ui.Output(errProjectMode, terminal.WithErrorStyle())
return ErrSentinel
}
c.refProject = &pb.Ref_Project{
Project: c.cfg.Project,
}
}
}

Yeah, 'cause we load the config earlier on and fail if it's not there before checking on flagProject. I'll remove it.

I'm pretty sure this doesn't have weird downstream effects, but I'm 100% sure that we're not depending on refProject being nil to do special project-agnostic behavior somewhere.
@@ -240,6 +240,7 @@ func (c *baseCommand) Init(opts ...Option) error {
match := reAppTarget.FindStringSubmatch(c.args[0])
if match != nil {
// Set our refs
c.refProject = &pb.Ref_Project{Project: match[1]}
Copy link
Contributor

Choose a reason for hiding this comment

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

Without this, running waypoint exec bin/bash causes a panic.

Even with this, it's not great behavior, because it parses bin as the project and bash as the app, but it's the same behavior as 0.5.2 and we can address it later.

@mitchellh
Copy link
Contributor

Huh, I wonder when this started happening. I'd be curious to run a bisect and figure out what caused this because this obviously didn't crash at some point. I don't use the exec or logs commands often so it could be awhile, but I'm curious from a learning perspective what caused this.

@krantzinator
Copy link
Contributor Author

krantzinator commented Oct 20, 2021

Huh, I wonder when this started happening. I'd be curious to run a bisect and figure out what caused this because this obviously didn't crash at some point. I don't use the exec or logs commands often so it could be awhile, but I'm curious from a learning perspective what caused this.

This started happening after #2413 . So we are fixing a couple of logic pathways from that. Some of the if logic statements weren't all encompassing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Certain commands result in panics
4 participants