/snapcraft

added support for uploading snap packages directly to the Ubuntu Store. #197

Merged
merged 1 commit into from Jan 27, 2016

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
4 participants
@ricardokirkner @kyrofa @elopio @sergiusens
@ricardokirkner
Contributor

ricardokirkner commented Dec 23, 2015

  • added login command to fetch credentials necessary to establish an
    authenticated session for the upload process
  • added logout command to clear stored credentials
  • added upload command to upload a snap package to the store
setup.py
+ ],
+ tests_require=[
+ 'click_toolbelt>=0.4.0',
+ ],
@kyrofa

kyrofa Dec 23, 2015

Member

Does this need to be added to debian/control as well?

@kyrofa
Member

kyrofa commented Dec 23, 2015

Please add your new dependencies to the .travis.yml file's installation step.

@kyrofa kyrofa added the enhancement label Dec 23, 2015

snapcraft/commands/upload.py
+ """Upload snap package to the Ubuntu Store."""
+
+ # make sure the full lifecycle is executed
+ snap = lifecycle.execute('strip')
@sergiusens

sergiusens Jan 4, 2016

Collaborator

you probably want to run the snap command here. This produces no snap.

Maybe some code reorg is needed if the snap command becomes common and just calling snap.main() is to cumbersome (or ugly).

@elopio
Member

elopio commented Jan 5, 2016

Thanks a lot Ricardo!
This needs integration tests, maybe just for the successful paths of login, logout and upload. Does this toolbelt have an easy way to patch the ip of the server so we can fake it?
Let me know if you need a hand, or if you have a better idea of how to test this. The integration tests are written here: https://github.com/ubuntu-core/snapcraft/tree/master/integration_tests

It would also be nice to upload a real snap to the staging server, maybe once every night, but that would require us to keep some credentials in jenkins. I'll discuss about it with @fgimenez.
integration_tests/test_upload.py
+ 'stage to create it.', output)
+ self.assertIn('Uploading {}'.format(snap_file_path), output)
+ self.assertIn('Package scan completed.', output)
+ self.assertIn('{} upload failed'.format(snap_file_path), output)
@elopio

elopio Jan 11, 2016

Member

Yes, that's what I had in mind.
I'd prefer to put the whole expected message in a var, maybe as a regex if there are things you want to ignore, and then make one assertion for the output, but that's just a nit.
What I think we need now is to check successful scenarios with a fake server.

integration_tests/test_upload.py
@@ -0,0 +1,50 @@
+# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*-
+#
+# Copyright (C) 2015 Canonical Ltd
@elopio

elopio Jan 11, 2016

Member

s/2015/2016.

integration_tests/test_upload.py
+ self.assertThat(snap_file_path, FileExists())
+
+ self.assertIn('Snap assemble_1.0_amd64.snap not found. Running snap '
+ 'stage to create it.', output)
@sergiusens

sergiusens Jan 27, 2016

Collaborator

can you s/stage to/step to to avoid confusion with the stage step?

runtests.sh
# mccabe in 'warning' mode as we have high complexity
mccabe_list=
- for unit in $(find snapcraft -type f -name '*.py')
+ for unit in $(find snapcraft -type f -name '*.py' -not -path 'snapcraft/vendor/*')
@sergiusens

sergiusens Jan 27, 2016

Collaborator

can we remove -not -path 'snapcraft/vendor/*' ?

runtests.sh
@@ -70,7 +65,7 @@ run_static_tests(){
run_unit_tests(){
if which python3-coverage >/dev/null 2>&1; then
python3-coverage erase
- python3-coverage run --branch --source snapcraft -m unittest discover -s snapcraft -t .
+ python3-coverage run --branch --source snapcraft --omit 'snapcraft/vendor/*' -m unittest discover -s snapcraft -t .
@sergiusens

sergiusens Jan 27, 2016

Collaborator

can we remove --omit 'snapcraft/vendor/*' ?

snapcraft/commands/upload.py
+
+ if not os.path.exists(snap_name):
+ logger.info(
+ 'Snap {} not found. Running snap stage to create it.'.format(
@sergiusens

sergiusens Jan 27, 2016

Collaborator

s/snap stage/snap step please

snapcraft/commands/upload.py
+
+def main(argv=None):
+ """Upload snap package to the Ubuntu Store."""
+
@sergiusens

sergiusens Jan 27, 2016

Collaborator

For help to work, we need this here:

argv = argv if argv else []
args = docopt(__doc__, argv=argv)
snapcraft/commands/logout.py
+
+def main(argv=None):
+ """Forget credentials for Ubuntu One SSO."""
+
@sergiusens

sergiusens Jan 27, 2016

Collaborator

For help to work, we need this here:

argv = argv if argv else []
args = docopt(__doc__, argv=argv)
snapcraft/commands/login.py
+
+
+def main(argv=None):
+ """Authenticates session against Ubuntu One SSO."""
@sergiusens

sergiusens Jan 27, 2016

Collaborator

For help to work, we need this here:

argv = argv if argv else []
args = docopt(__doc__, argv=argv)
@sergiusens
Collaborator

sergiusens commented Jan 27, 2016

Looks pretty good, the only thing I think is missing is some sort of upload progress

$ ../../bin/snapcraft upload --help
Uploading shout_0.52.0_amd64.snap
Uploading files...
Starting new HTTPS connection (1): upload.apps.ubuntu.com
Uploading new version...
Starting new HTTPS connection (1): myapps.developer.ubuntu.com
Package submitted to https://myapps.developer.ubuntu.com/dev/api/click-package-upload/shout/
Checking package status...
... retrying in 5 seconds
Resetting dropped connection: myapps.developer.ubuntu.com
Package scan completed.
Upload did not complete.
Some errors were detected:

The uploaded version (0.52.0) has already been uploaded.


shout_0.52.0_amd64.snap upload failed

Thanks for working on this btw, we really appreciate the contribution!!!!
@sergiusens
Collaborator

sergiusens commented Jan 27, 2016

I'm sorry to say the error seen here makes little sense to me, it might just be a network error, in any case, mind doing a:

git checkout master
git pull
git checkout <this-branch>
git rebase master
# also add the lp bug this is fixing (add "LP: #1538657" on a single line)
git commit --amend
git push --force <your-origin>
@ricardokirkner
added support for uploading snap packages directly to the Ubuntu Store. 
LP: #1538657

- added login command to fetch credentials necessary to establish an
  authenticated session for the upload process
- added logout command to clear stored credentials
- added upload command to upload a snap package to the store
- added support for reading and writing configuration from disk
- added basic integration test for upload command
- use flake8 instead of pyflakes

All checks have passed

2 successful checks
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
coverage/coveralls — Coverage increased (+1.9%) to 93.214%
Details
2329098
@sergiusens
Collaborator

sergiusens commented Jan 27, 2016

OK, I've decided to let this in as is, thanks.

I've logged https://bugs.launchpad.net/snapcraft/+bug/1538692

sergiusens added a commit that referenced this pull request Jan 27, 2016

@sergiusens
Merge pull request #197 from ricardokirkner/upload-command 
added support for uploading snap packages directly to the Ubuntu Store.
75d3346

@sergiusens sergiusens merged commit 75d3346 into snapcore:master Jan 27, 2016
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+1.9%) to 93.214%
Details

kalikiana pushed a commit to kalikiana/snapcraft that referenced this pull request Apr 6, 2017

@sergiusens
Merge pull request #197 from ricardokirkner/upload-command 
added support for uploading snap packages directly to the Ubuntu Store.
755224f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment