-
Notifications
You must be signed in to change notification settings - Fork 24
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.
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
feat(quarantine): add quarantine add/rm command #80
Conversation
DavidS-cloud
commented
May 19, 2022
…t had Quarantine enabled
``` cloudsmith-cli on SPOCK-360/add-quarantine-cmd [+?⇡] via 🐍 v3.7.2 (.venv) on ☁️ (eu-west-1) ❯ python -m cloudsmith_cli quarantine rm cloudsmith/testing-private/CjUgS7XxKI8j-Nnd Removing testing-private/CjUgS7XxKI8j-Nnd from quarantine... OK cloudsmith-cli on SPOCK-360/add-quarantine-cmd [+?⇡] via 🐍 v3.7.2 (.venv) on ☁️ (eu-west-1) ❯ python -m cloudsmith_cli quarantine rm cloudsmith/testing-private/CjUgS7XxKI8j-Nnd Removing testing-private/CjUgS7XxKI8j-Nnd from quarantine... ERROR Failed quarantine! (status: 400 - Bad Request) Detail: You cannot restore a package that's not quarantined. cloudsmith-cli on SPOCK-360/add-quarantine-cmd [+?⇡] via 🐍 v3.7.2 (.venv) on ☁️ (eu-west-1) ❯ python -m cloudsmith_cli quarantine add cloudsmith/testing-private/CjUgS7XxKI8j-Nnd Adding testing-private/CjUgS7XxKI8j-Nnd to quarantine... OK cloudsmith-cli on SPOCK-360/add-quarantine-cmd [+?⇡] via 🐍 v3.7.2 (.venv) on ☁️ (eu-west-1) ❯ python -m cloudsmith_cli quarantine add cloudsmith/testing-private/CjUgS7XxKI8j-Nnd Adding testing-private/CjUgS7XxKI8j-Nnd to quarantine... ERROR Failed quarantine! (status: 400 - Bad Request) Detail: You cannot quarantine this package at this point in time. cloudsmith-cli on SPOCK-360/add-quarantine-cmd [+?⇡] via 🐍 v3.7.2 (.venv) on ☁️ (eu-west-1) ❯ python -m cloudsmith_cli quarantine rm cloudsmith/testing-private/CjUgS7XxKI8j-Nnd Removing testing-private/CjUgS7XxKI8j-Nnd from quarantine... OK cloudsmith-cli on SPOCK-360/add-quarantine-cmd [+?⇡] via 🐍 v3.7.2 (.venv) on ☁️ (eu-west-1) ❯ python -m cloudsmith_cli quarantine rm cloudsmith/testing-private/CjUgS7XxKI8j-Nnd Removing testing-private/CjUgS7XxKI8j-Nnd from quarantine... ERROR Failed quarantine! (status: 400 - Bad Request) Detail: You cannot restore a package that's not quarantined. cloudsmith-cli on SPOCK-360/add-quarantine-cmd [+?⇡] via 🐍 v3.7.2 (.venv) on ☁️ (eu-west-1) ❯ ```
return wrapper | ||
|
||
|
||
@main.group(cls=command.AliasGroup, aliases=["qu"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar blocks of code found in 2 locations. Consider refactoring.
return add_quarantine(*args, **kwargs) | ||
|
||
|
||
def remove_quarantine(ctx, opts, owner_repo_package, page, page_size): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar blocks of code found in 2 locations. Consider refactoring.
See the help for subcommands for more information on each. | ||
""" | ||
|
||
def add_quarantine(ctx, opts, owner_repo_package, page, page_size): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar blocks of code found in 2 locations. Consider refactoring.
return add_quarantine(*args, **kwargs) | ||
|
||
|
||
def remove_quarantine(ctx, opts, owner_repo_package, page, page_size): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function remove_quarantine
has 5 arguments (exceeds 4 allowed). Consider refactoring.
See the help for subcommands for more information on each. | ||
""" | ||
|
||
def add_quarantine(ctx, opts, owner_repo_package, page, page_size): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function add_quarantine
has 5 arguments (exceeds 4 allowed). Consider refactoring.
54a7e9d
to
fadd9ba
Compare
See also #81 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just one small opinion re the qu
shorthand; feel free to get overrule though from @lskillen
README.md
Outdated
@@ -73,6 +73,9 @@ The CLI currently supports the following commands (and sub-commands): | |||
- `ruby`: Push (upload) a new Ruby package upstream. | |||
- `terraform`: Push (upload) a new Terraform package upstream. | |||
- `vagrant`: Push (upload) a new Vagrant package upstream. | |||
- `quarantine`|`qu`: Manage quarantined packages in a repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not 100% sure on the shorthand here...I get it because quarantine is a long word that I spell incorrectly very regularly 😂 But the qu
also forms part of quota
below. quar
maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, yeah, that makes sense 😅
Might want to skip the shorthand completly tho: https://www.urbandictionary.com/define.php?term=quar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha, yes! 😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Err 😂 Yeah, only list aliases rather than shorthand (because shorthand is automatically handled by python-click). If you want a cool alias, I'd go for "jail" but otherwise, feel free to move along. :-)
return wrapper | ||
|
||
|
||
@main.group(cls=command.AliasGroup, aliases=["block"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar blocks of code found in 2 locations. Consider refactoring.
Code Climate has analyzed commit 9448f54 and detected 6 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 0.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 0.9% (0.0% change). View more on Code Climate. |