This seems pretty unobjectionable to me. I guess there's a bit of a UX question about the flag name -- my first instinct is --port/-p (as in fly -t target login -c <concourse url> --port <token callback port>), but that conceptually overlaps with --pipeline/-p from other commands. Not the end of the world, but maybe there's a smarter choice.

Probably no surprise to you @richmondwang, but everybody's busy and this is the kind of work that doesn't really fit into the priorities in the Roadmap being pursued by the full-time contributors. So I'll just try to give some technical guidance here for anybody who decides to pick it up (maybe you!).

The change could reasonably be driven by adding a context similar to this one in the login integration tests, but where flyCmd gets called with the extra flag for the fixed port and we don't need to dynamically parse the token listener port - we can just assert that there's a server listening at exactly the port we expect.

The implementation should probably involve adding a new field, like TokenListenerPort int to the LoginCommand struct, and the behaviour change should happen around here, changing the :0 to the TokenListenerPort value.

This is a fairly advanced use-case, so I don't think we need to heavily document it, but the docs already have a decently-detailed section for fly login, so it wouldn't hurt to describe this use-case there. That would take the form of a patch to this section of the docs source code.

EDIT: hey, what about -s for the token listener port? I feel like I've used other CLIs that, in the case that they can run a server, usually use -s for that server's configuration.

hey, what about -s for the token listener port? I feel like I've used other CLIs that, in the case that they can run a server, usually use -s for that server's configuration.

Yes, I think -s is good. But I was also thinking of adding both --token-host and --token-port as long-named command options (only?), because we need to set the host properly too (e.i. inside docker). What do you think?

... for anybody who decides to pick it up (maybe you!)

I will try to get some time for this if no one can take it.

Probably no surprise to you @richmondwang, but everybody's busy and this is the kind of work that doesn't really fit into the priorities in the Roadmap being pursued by the full-time contributors....

Nope, not a surprise at all :P and I agree with priorities. 👍

@richmondwang Just FYI, #4708 may help your use case, with that, you may set_pipeline from a pipeline without run fly login.

@richmondwang Just FYI, #4708 may help your use case, with that, you may set_pipeline from a pipeline without run fly login.

Thanks for this @evanchaoli , this feature is very useful for us too...

I have a concern though (more kind of suggestion hehe).. Given here that I generate the pipeline on the run using some scripting, I want to be able to check the diff to avoid unwanted changes. For the feature you wrote (thank you, by the way), I am not really sure I am able to do it. If we were to add a flag as a param to the set_pipeline, let's say apply_pipeline: false or show_diff_only: true without really setting the pipeline itself, then It will be useful in my case here. And we could just add another job, with the same params as the former set_pipeline configuration, but with apply_pipeline: true, which this time really applies the pipeline.

kinda like terraform plan, and terraform apply

PS.
Also, I think this enhancement still makes sense, as it has different points of the #4708 feature.

@richmondwang Thanks for the suggestion. With #4708, set_pipeline will show diff to build logs, if there is no diff, it just terminates, otherwise go ahead to set the new pipeline.

It should be easy to set a flag of show_diff_only, even dump diff to a file, but I don't want to add anything more to #4708, once #4708 is merged, I may create a new PR to support the flag.

To confirm, what behavior do you expect from show_diff_only? If you want to get diff lines from a pipeline task?

Im not sure if we should continue this on the PR or the RFC... but...

To confirm, what behavior do you expect from show_diff_only? If you want to get diff lines from a pipeline task?

I want to be able to see the diff and be able to decide wether I want to set the pipeline with the new config or abort and not set the pipeline.

Beep boop! This issue has been idle for long enough that it's time to check in and see if it's still important.

If it is, what is blocking it? Would anyone be interested in submitting a PR or continuing the discussion to help move things forward?

If no activity is observed within the next week, this issue will be exterminated closed, in accordance with our stale issue process.