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.

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

Implemented --query flag in conan upload (#2445) #3377

Merged
merged 4 commits into from
Aug 22, 2018
Merged

Implemented --query flag in conan upload (#2445) #3377

merged 4 commits into from
Aug 22, 2018

Conversation

joechrisellis
Copy link
Contributor

@joechrisellis joechrisellis commented Aug 20, 2018

You can now use the --query flag in the conan upload command like you
would in conan remove. In other words, you can use a query to specify
which packages will be uploaded to the server.

Will update documentation tonight.

You can now use the --query flag in the `conan upload` command like you
would in `conan remove`. In other words, you can use a query to specify
which packages will be uploaded to the server.
@ghost ghost added the contributor pr label Aug 20, 2018
Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

This feature is nice, I think it makes sense and could be merged. However, it would require to add some test of this new feature. Please try to to having a look to existing tests (look for those using TestServer) and ask for help if necessary! Many thanks!

@@ -728,8 +728,9 @@ def search_packages(self, reference, query=None, remote_name=None, outdated=Fals
return recorder.get_info()

@api_method
def upload(self, pattern, package=None, remote_name=None, all_packages=False, force=False,
confirm=False, retry=2, retry_wait=5, skip_upload=False, integrity_check=False,
def upload(self, pattern, package=None, query=None, remote_name=None,
Copy link
Member

Choose a reason for hiding this comment

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

I would say add the new argument in the last position, less likely to break to users of the API

@joechrisellis
Copy link
Contributor Author

Would be happy to take a look at creating tests, will do this asap. :)

@memsharded memsharded added this to the 1.7 milestone Aug 20, 2018
Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

Tests looks very good too, great job!

@@ -123,13 +167,14 @@ def gzopen_patched(name, mode="r", fileobj=None, compresslevel=None, **kwargs):
self.assertIn("ERROR: Error gzopen conan_package.tgz", client.out)

export_folder = client.client_cache.package(package_ref)
print(export_folder)
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove print

Copy link
Member

@danimtb danimtb left a comment

Choose a reason for hiding this comment

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

Great contribution! Thanks a lot 😃

_PATTERN_OR_REFERENCE_HELP = ("Pattern or package recipe reference, e.g., 'boost/*', "
"'MyPackage/1.2@user/channel'")
_PATTERN_OR_REFERENCE_HELP = ("Pattern or package recipe reference, e.g., '{}', "
"'{}'".format(_REFERENCE_EXAMPLE, _PATTERN_EXAMPLE))
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather use %s to format the string to be consistent with Conan codebase

_QUERY_HELP = ("Packages query: 'os=Windows AND (arch=x86 OR compiler=gcc)'. The "
"'pattern_or_reference' parameter has to be a reference: MyPackage/1.2@user/channel")
_QUERY_HELP = ("Packages query: '{}'. The 'pattern_or_reference' parameter has "
"to be a reference: {}".format(_QUERY_EXAMPLE, _REFERENCE_EXAMPLE))
Copy link
Member

Choose a reason for hiding this comment

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

Same thing here

* .format replaced with old-style Python 2 formatting for consistency.
* Removed extraneous print statement.
@joechrisellis
Copy link
Contributor Author

Removed extraneous print statement and removed new-style formatting for consistency. :)

@joechrisellis
Copy link
Contributor Author

Made changes in conan-io/docs for the new option -- see: conan-io/docs#772

@memsharded memsharded merged commit 4648e9b into conan-io:develop Aug 22, 2018
@ghost ghost removed the stage: review label Aug 22, 2018
@memsharded
Copy link
Member

Merged! Will be in conan 1.7. Thanks again for your contribution :)

@joechrisellis
Copy link
Contributor Author

Awesome, and no problem :D

grisumbras pushed a commit to grisumbras/conan that referenced this pull request Dec 27, 2018
…3377)

* Implemented --query flag in `conan upload` (conan-io#2445)

You can now use the --query flag in the `conan upload` command like you
would in `conan remove`. In other words, you can use a query to specify
which packages will be uploaded to the server.

* Moved query to final argument to prevent breakage for API users

* Added tests for `conan upload --query`

* Made changes from code review.

* .format replaced with old-style Python 2 formatting for consistency.
* Removed extraneous print statement.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding a -q option to conan upload would help
4 participants