Skip to content

Fix asking for username/password in Publisher#5085

Closed
NiklasRosenstein wants to merge 5 commits into
masterfrom
unknown repository
Closed

Fix asking for username/password in Publisher#5085
NiklasRosenstein wants to merge 5 commits into
masterfrom
unknown repository

Conversation

@NiklasRosenstein
Copy link
Copy Markdown

Fixes #4790

Somewhere between Cleo 0.8.0 and 1.0.0a4, the IO.ask() and IO.ask_hidden() methods were removed. I added Publisher.ask() which is almost equivalent to Cleo's Command.ask(), only that it also accepts a hidden argument.

Comment thread src/poetry/publishing/publisher.py Outdated
@NiklasRosenstein
Copy link
Copy Markdown
Author

Hey @neersighted , what do you think of it now?

@NiklasRosenstein
Copy link
Copy Markdown
Author

Pinging @radoering as the last committer on Poetry hoping to get some momentum in here. 🙃

@radoering
Copy link
Copy Markdown
Member

radoering commented Feb 25, 2022

Two remarks:

  1. Is there a reason why the import of Question is local?
  2. I wonder if it is feasible to write a test in order to detect possible API changes in the future?

@neersighted
Copy link
Copy Markdown
Member

Hey @neersighted , what do you think of it now?

Sorry, fell down the work hole -- I think @radoering should be able to get you over the finish line :)

@branchv branchv added this to the 1.2 milestone May 31, 2022
@branchv branchv added area/cli Related to the command line status/external-issue Issue is caused by external project (platform, dep, etc) Dependency labels May 31, 2022
@branchv
Copy link
Copy Markdown
Member

branchv commented Jun 2, 2022

@NiklasRosenstein sorry for the long back and forth, are you still interested in finishing this out with a test? You could follow the examples using inputs from test_init.py

No worries if not, I can help with this last step

@dimbleby
Copy link
Copy Markdown
Contributor

dimbleby commented Jun 21, 2022

Just spotted this after #5889 was merged. In that MR I removed this code altogether - which certainly fixes the bug that you were hitting.

Do you actually care about asking for username and password here? IMO if this is to happen it would be better on the Command, but deleting the code seems satisfactory to me, the command line accepts --password and --username already and I don't see any compelling reason to treat just these two parameters on just this one command exceptionally.

(Also this has been broken for so long that it's clear that no-one has been using the function!)

@neersighted
Copy link
Copy Markdown
Member

Just spotted this after #5889 was merged. In that MR I removed this code altogether - which certainly fixes the bug that you were hitting.

Do you actually care about asking for username and password here? IMO if this is to happen it would be better on the Command, but deleting the code seems satisfactory to me, the command line accepts --password and --username already and I don't see any compelling reason to treat just these two parameters on just this one command exceptionally.

(Also this has been broken for so long that it's clear that no-one has been using the function!)

I merged that PR in light of this one existing / planning to circle back to it. I do think that the interactive credential support is an anti-pattern and that given we've greatly improved support for non-interactive credentials there is no more need for it. Combine that with adaptation to newer Cleo versions being kludgy and I don't particular care to resurrect it.

That being said, @NiklasRosenstein if you disagree, please feel free to post a new version of this patch/re-open this PR, especially if you can come up with a more elegant design. @dimbleby's input looks good to me.

@github-actions
Copy link
Copy Markdown

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area/cli Related to the command line status/external-issue Issue is caused by external project (platform, dep, etc)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

'IO' object has no attribute 'ask'

5 participants