Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
ftp source support #900
Conversation
|
This will probably need to be fixed once #906 gets merged... |
|
Wondering why the integration test fails in autopkgtest with the |
3v1n0
referenced this pull request
in ubuntu-core/snappy-jenkins
Nov 18, 2016
Open
Export ftp_proxy on autopkgtests #210
|
Ok the connection timeout issue was due to missing For it to pass, right now it was enough to reuse the That can be removed once ubuntu-core/snappy-jenkins#210 will be applied, I guess. @elopio if you can give a look to both, it would be appreciated ;-) |
elopio
approved these changes
Nov 18, 2016
I left some minor naming comments, but I'll leave my +1 here anyway.
| @@ -64,6 +71,36 @@ def download_requests_stream(request_stream, destination, message=None): | ||
| progress_bar.finish() | ||
| +class UrllibDownlaoder(object): |
| + if self.progress_bar: | ||
| + self.progress_bar.finish() | ||
| + | ||
| + def _progress_cb(self, block_num, block_size, total_length): |
elopio
Nov 18, 2016
Member
I know I'm being a pita, but can you rename this to _progress_callback?
I have a hardtime with acronyms because my brain is not yet fast enough in english to understand them. I always have to spent a few minutes parsing it. That's the only thing I hate about go, and that's why I love that python code tends to be verbose :)
| @@ -109,6 +111,22 @@ def setUp(self): | ||
| self.useFixture(fixture_setup.FakeParts()) | ||
| +class FakeFileHttpServerBasedTestCase(TestCase): |
|
@3v1n0 that sounds like a thing to add to our autopkgtest setup. Can you talk to pitti about it? |
|
Ok @elopio, changed the naming as suggested. As for the |
martinpitt
commented
Nov 18, 2016
|
Canonical datacenter's proxy does not allow ftp, sorry. You have to use http, or skip the test. |
|
@3v1n0 nop, sorry, I forgot to reply to that. Last week we stopped using that jenkins server, and instead we are now sending the PRs directly to the autopkgtest infrastructure: https://wiki.ubuntu.com/ProposedMigration/AutopkgtestInfrastructure#Integration_with_GitHub_and_GitLab_pull.2Fmerge_requests |
|
@martinpitt but... If I set the |
|
@sergiusens any moment for looking at this? |
3v1n0
added some commits
Nov 12, 2016
All checks have passed
|
Rebased on latest master |
sergiusens
approved these changes
Dec 1, 2016
If requests-ftp where available on the distro I would of told you to use that. Nice work.
sergiusens
merged commit eba7dd2
into
snapcore:master
Dec 1, 2016
5 checks passed
pushed a commit
to kalikiana/snapcraft
that referenced
this pull request
Apr 6, 2017
3v1n0 commentedNov 12, 2016
As requested on bug https://bugs.launchpad.net/snappy/+bug/1602323
This adds support for FTP sources, using urllib was the quickest choice, and with some refactor of the indicator module, adding a progressbar was possible too. If using ftplib or others is preferred let me know, but I went for the fastest and safer way.
In theory this path could be used also for http(s) downloads, but not to change what we've done so far, I've left that job to the nicer
requestslibrary.requestsbeing an HTTP only library has no native suppot for this, while trying to use something like requests-ftp I don't think is a choice (looking quite immature).