Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add --skip-existing to publish #2812

Merged
merged 2 commits into from
Mar 29, 2022
Merged

add --skip-existing to publish #2812

merged 2 commits into from
Mar 29, 2022

Conversation

davidszotten
Copy link
Contributor

@davidszotten davidszotten commented Aug 14, 2020 •
edited
Loading

Pull Request Check List

Resolves: #2254

WIP to check approach.

  • Added tests for changed code.
  • Updated documentation for changed code.

Questions

  • does this seem reasonable?
  • twine is apache 2 licensed, is copying their impl ok?
  • should we consider bundling together the "options" (dry_run and skip_existing) into an options object or similar to have fewer things to thread through all the functions?

/cc @abn

@davidszotten davidszotten force-pushed the publish_skip_existing branch 3 times, most recently from 86d7998 to a530706 Compare August 17, 2020 13:30
@abn abn marked this pull request as draft August 30, 2020 13:40
@abn abn changed the base branch from develop to master August 30, 2020 13:40
@davidszotten
Copy link
Contributor Author

hey @abn . did you mark this as wip/draft? what is the next step here?

@davidszotten davidszotten marked this pull request as ready for review April 28, 2021 12:14
@sonarcloud
Copy link

sonarcloud bot commented Jul 9, 2021

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
5.9% 5.9% Duplication

@davidszotten
Copy link
Contributor Author

@sdispater what's the policy for the sonarcloud report? apparently i'm adding a "code smell" though i'm just trying to be consistent with the surrounding code

@davidszotten davidszotten changed the title [WIP] add --skip-existing to publish add --skip-existing to publish Jul 14, 2021
@davidszotten davidszotten force-pushed the publish_skip_existing branch 2 times, most recently from 121ce51 to d66395f Compare July 22, 2021 09:49
@cob16
Copy link

cob16 commented Aug 27, 2021

How is this PR going? Is this ready to merge now?

@IsmaelMartinez
Copy link

Someone from the project needs to review it. LGTM

@IsmaelMartinez
Copy link

@sdispater or @abn ? do you think you can assign someone to review this and/or review it so we can get this feature? Much appreciated and thanks a lot for this awesome project.

@czomo
Copy link

czomo commented Mar 24, 2022

Any chance this will be reviewed in near future?

@czomo
Copy link

czomo commented Mar 26, 2022

@finswimmer and @radoering could you help us here?

radoering
radoering requested changes Mar 26, 2022
View reviewed changes
Copy link
Member

@radoering radoering left a comment

Choose a reason for hiding this comment

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

At first, thanks for your contribution and your patience. In my opinion, this option is useful. (I even had the use case myself once.)

Regarding the license question, I think that is difficult to answer. APACHE license allows using the code, but demands to include the license notice. In order to avoid that, I'd prefer to rewrite the code. Since the function is not that big, there are not so many options. (It even may not be necessary because it's so small. I'm not a licensing expert.)

Anyway, I made some suggestions. Feel free to adopt or implement your own ideas.

I just reviewed the _ignore_existing function for now and will continue later.

src/poetry/publishing/uploader.py Outdated Show resolved Hide resolved
src/poetry/publishing/uploader.py Outdated Show resolved Hide resolved
src/poetry/publishing/uploader.py Outdated Show resolved Hide resolved
radoering
radoering requested changes Mar 26, 2022
View reviewed changes
src/poetry/publishing/uploader.py Outdated Show resolved Hide resolved
radoering
radoering requested changes Mar 27, 2022
View reviewed changes
src/poetry/publishing/uploader.py Outdated Show resolved Hide resolved
src/poetry/publishing/uploader.py Outdated Show resolved Hide resolved
src/poetry/publishing/uploader.py Outdated Show resolved Hide resolved
src/poetry/publishing/uploader.py Outdated Show resolved Hide resolved
src/poetry/publishing/uploader.py Outdated Show resolved Hide resolved
tests/publishing/test_uploader.py Outdated Show resolved Hide resolved
tests/console/commands/test_publish.py Show resolved Hide resolved
src/poetry/publishing/uploader.py Outdated Show resolved Hide resolved
radoering
radoering requested changes Mar 28, 2022
View reviewed changes
src/poetry/publishing/uploader.py Outdated Show resolved Hide resolved
tests/publishing/test_uploader.py Outdated Show resolved Hide resolved
src/poetry/publishing/uploader.py Outdated Show resolved Hide resolved
tests/console/commands/test_publish.py Outdated Show resolved Hide resolved
@davidszotten
add --skip-existing to publish
d821a43 
add `--skip-existing` option to publish command to continue if an upload
fails due to the file already existing. this is helpful in e.g. ci
settings. resolves python-poetry#2254
@radoering radoering merged commit cf8999f into python-poetry:master Mar 29, 2022
@davidszotten
Copy link
Contributor Author

thank you for all your help @radoering !

davidszotten added a commit to davidszotten/poetry that referenced this pull request Mar 29, 2022
@davidszotten
add changelog for python-poetry#2812
7bf190a
@davidszotten davidszotten mentioned this pull request Mar 29, 2022
Closed
@kasteph kasteph mentioned this pull request May 30, 2022
Merged
@radoering radoering mentioned this pull request Aug 12, 2022
Closed
2 tasks
@github-actions GitHub Actions
Copy link

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.
Reviewers

@IsmaelMartinez IsmaelMartinez IsmaelMartinez left review comments

@radoering radoering radoering approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add --skip-existing option to poetry publish
5 participants
@davidszotten @cob16 @IsmaelMartinez @czomo @radoering