-
Notifications
You must be signed in to change notification settings - Fork 12
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
bump version to 0.1.3 to permit release on CI #72
Merged
Merged
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
e4f5efb
bump version to 0.1.3 to permit release on CI
b8ec8fd
bump version to 0.1.3 to permit release on CI
0bfd48b
Update RELEASE_NOTES.md
Tianhao-Gu 8d74391
added history to RELEASE_NOTES.md
f8111b1
Update RELEASE_NOTES.md
qzzhang f2d6a64
Update RELEASE_NOTES.md
jsfillman ea9b2e5
handle noclobber error for os.mkdir()
20a2260
Update RELEASE_NOTES.md
jsfillman 1fd3435
Merge branch 'dcchivian-patch-1' into jsfillman-patch-1
85caefa
Merge pull request #74 from kbaseapps/jsfillman-patch-1
9fb2b7f
build local FTP server for tests
Tianhao-Gu b756685
remove unused argument
Tianhao-Gu e3e2797
Merge pull request #75 from Tianhao-Gu/fix_ftp_test
Tianhao-Gu e6376d1
Moved the change notes from under 1.1 to under 1.2
qzzhang 4d039e4
Fix tests failing due to shock -> blobstore cutover
MrCreosote dd6a938
Remove .travis.yml
MrCreosote File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,17 @@ | ||
#0.1.3 | ||
- tidying for github consistency | ||
- Added Actions workflow for KB SDK tests | ||
- Added Dependabot and LGTM configurations | ||
- Updated `README.md` to include standard build, coverage, and LGTM badging. | ||
|
||
#0.1.2 | ||
- deprecate handle service with handle service 2 | ||
- replace the colon(:) [that was reported to have caused download error for Windows users] with underscore in shock filenames | ||
|
||
#0.1.1 | ||
- close no longer used sockets. | ||
|
||
#0.1.0 | ||
|
||
- shock attributes are now ignored on upload. In a future release they will be removed altogether | ||
and specifying attributes for upload will be an error. | ||
- shock indexes and attributes are no longer copied during a copy or ownership operation. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,7 @@ service-language: | |
python | ||
|
||
module-version: | ||
0.1.2 | ||
0.1.3 | ||
|
||
owners: | ||
[rsutormin, msneddon, gaprice, scanon, tgu2] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
import ftplib | ||
import gzip | ||
import os.path | ||
import errno | ||
import shutil | ||
import tarfile | ||
import tempfile | ||
|
@@ -15,6 +16,11 @@ | |
import requests | ||
import semver | ||
|
||
from pyftpdlib.authorizers import DummyAuthorizer | ||
from pyftpdlib.handlers import FTPHandler | ||
from pyftpdlib.servers import ThreadedFTPServer | ||
import threading | ||
|
||
from DataFileUtil.DataFileUtilImpl import DataFileUtil, ShockException, HandleError, WorkspaceError | ||
from DataFileUtil.DataFileUtilServer import MethodContext | ||
from DataFileUtil.authclient import KBaseAuth as _KBaseAuth | ||
|
@@ -64,6 +70,28 @@ def setUpClass(cls): | |
wsName = "test_DataFileUtil_" + str(suffix) | ||
cls.ws_info = cls.ws.create_workspace({'workspace': wsName}) | ||
|
||
cls.ftp_domain = 'localhost' | ||
cls.ftp_port = 21 | ||
thread = threading.Thread(target=cls.start_ftp_service, | ||
args=(cls.ftp_domain, cls.ftp_port)) | ||
thread.daemon = True | ||
thread.start() | ||
time.sleep(5) | ||
|
||
@classmethod | ||
def start_ftp_service(cls, domain, port): | ||
|
||
print('starting ftp service') | ||
authorizer = DummyAuthorizer() | ||
authorizer.add_anonymous(os.getcwd(), perm='elradfmwMT') | ||
|
||
handler = FTPHandler | ||
handler.authorizer = authorizer | ||
|
||
address = (domain, port) | ||
with ThreadedFTPServer(address, handler) as server: | ||
server.serve_forever() | ||
|
||
@classmethod | ||
def tearDownClass(cls): | ||
if hasattr(cls, 'ws_info'): | ||
|
@@ -225,7 +253,7 @@ def test_unpack_large_zip(self): | |
zip_filename = 'large_file.txt.zip' | ||
tmp_dir = os.path.join(self.cfg['scratch'], 'unpacklargeziptest') | ||
if not os.path.exists(tmp_dir): | ||
os.makedirs(tmp_dir) | ||
os.makedirs(tmp_dir) | ||
zip_file_path = os.path.join(tmp_dir, zip_filename) | ||
txt_file_path = os.path.join(tmp_dir, txt_filename) | ||
|
||
|
@@ -547,7 +575,7 @@ def fail_unpack(self, file_path, unpack, error): | |
self.fail_download({'shock_id': sid, | ||
'file_path': td, | ||
'unpack': unpack}, | ||
error) | ||
error) | ||
self.delete_shock_node(sid) | ||
|
||
def test_uncompress_on_archive(self): | ||
|
@@ -660,7 +688,7 @@ def test_pack_large_zip(self): | |
filename = 'large_file.txt' | ||
tmp_dir = os.path.join(self.cfg['scratch'], 'packlargeziptest') | ||
if not os.path.exists(tmp_dir): | ||
os.makedirs(tmp_dir) | ||
os.makedirs(tmp_dir) | ||
file_path = os.path.join(tmp_dir, filename) | ||
|
||
size_3GB = 3 * 1024 * 1024 * 1024 | ||
|
@@ -730,7 +758,12 @@ def test_download_existing_dir(self): | |
{'file_path': 'data/file1.txt'})[0] | ||
sid = ret1['shock_id'] | ||
d = 'foobarbazbingbang' | ||
os.mkdir(d) | ||
try: | ||
os.mkdir(d) | ||
except OSError as exc: | ||
if exc.errno != errno.EEXIST: | ||
raise | ||
pass | ||
ret2 = self.impl.shock_to_file(self.ctx, {'shock_id': sid, | ||
'file_path': d + '/foo', | ||
} | ||
|
@@ -767,20 +800,7 @@ def test_download_err_node_not_found(self): | |
'Error downloading file from shock node ' + | ||
'79261fd9-ae10-4a84-853d-1b8fcd57c8f23: Node not found', | ||
exception=ShockException) | ||
|
||
def test_download_err_node_has_no_file(self): | ||
# test attempting download on a node without a file. | ||
res = requests.post( | ||
self.shockURL + '/node/', | ||
headers={'Authorization': 'OAuth ' + self.token}).json() | ||
self.fail_download( | ||
{'shock_id': res['data']['id'], | ||
'file_path': 'foo' | ||
}, | ||
'Node {} has no file'.format(res['data']['id']), | ||
exception=ShockException) | ||
self.delete_shock_node(res['data']['id']) | ||
|
||
|
||
def test_download_err_no_node_provided(self): | ||
self.fail_download( | ||
{'shock_id': '', | ||
|
@@ -855,7 +875,7 @@ def test_copy_err_node_not_found(self): | |
{'shock_id': '79261fd9-ae10-4a84-853d-1b8fcd57c8f23'}, | ||
'Error copying Shock node ' + | ||
'79261fd9-ae10-4a84-853d-1b8fcd57c8f23: ' + | ||
'err@node_CreateNodeUpload: not found', | ||
'Invalid copy_data: invalid UUID length: 37', | ||
exception=ShockException) | ||
|
||
def test_copy_err_no_node_provided(self): | ||
|
@@ -1288,7 +1308,6 @@ def test_download_staging_file_archive_file(self): | |
self.assertEqual(os.stat(os.path.join("data", "zip1.zip")).st_size, | ||
os.stat(ret1['copy_file_path']).st_size) | ||
|
||
|
||
def fail_download_web_file(self, params, error, exception=ValueError, startswith=False): | ||
with self.assertRaises(exception) as context: | ||
self.impl.download_web_file(self.ctx, params) | ||
|
@@ -1379,9 +1398,9 @@ def test_fail_download_web_file_ftp(self): | |
|
||
invalid_input_params = { | ||
'download_type': 'FTP', | ||
'file_url': 'ftp://ftp.uconn.edu/48_hour/nonexist.txt'} | ||
'file_url': 'ftp://{}/{}'.format(self.ftp_domain, 'nonexist.txt')} | ||
error_msg = "File nonexist.txt does NOT exist in FTP path: " | ||
error_msg += "ftp.uconn.edu/48_hour" | ||
error_msg += self.ftp_domain + '/' | ||
self.fail_download_web_file(invalid_input_params, error_msg) | ||
|
||
def test_download_direct_link_uncompress_file(self): | ||
|
@@ -1593,18 +1612,16 @@ def test_download_ftp_link_uncompress_file(self): | |
|
||
fq_filename = "file1.txt" | ||
|
||
with ftplib.FTP('ftp.uconn.edu') as ftp_connection: | ||
with ftplib.FTP(self.ftp_domain) as ftp_connection: | ||
ftp_connection.login('anonymous', '[email protected]') | ||
ftp_connection.cwd("/48_hour/") | ||
|
||
if fq_filename not in ftp_connection.nlst(): | ||
fh = open(os.path.join("data", fq_filename), 'rb') | ||
ftp_connection.storbinary('STOR file1.txt', fh) | ||
fh.close() | ||
with open(os.path.join("data", fq_filename), 'rb') as fh: | ||
ftp_connection.storbinary('STOR {}'.format(fq_filename), fh) | ||
|
||
params = { | ||
'download_type': 'FTP', | ||
'file_url': 'ftp://ftp.uconn.edu/48_hour/file1.txt', | ||
'file_url': 'ftp://{}/{}'.format(self.ftp_domain, fq_filename), | ||
} | ||
|
||
ret1 = self.impl.download_web_file(self.ctx, params)[0] | ||
|
@@ -1618,18 +1635,16 @@ def test_download_ftp_link_compress_file(self): | |
|
||
fq_filename = "file1.txt.bz" | ||
|
||
with ftplib.FTP('ftp.uconn.edu') as ftp_connection: | ||
with ftplib.FTP(self.ftp_domain) as ftp_connection: | ||
ftp_connection.login('anonymous', '[email protected]') | ||
ftp_connection.cwd("/48_hour/") | ||
|
||
if fq_filename not in ftp_connection.nlst(): | ||
fh = open(os.path.join("data", fq_filename), 'rb') | ||
ftp_connection.storbinary('STOR file1.txt.bz', fh) | ||
fh.close() | ||
with open(os.path.join("data", fq_filename), 'rb') as fh: | ||
ftp_connection.storbinary('STOR {}'.format(fq_filename), fh) | ||
|
||
params = { | ||
'download_type': 'FTP', | ||
'file_url': 'ftp://ftp.uconn.edu/48_hour/file1.txt.bz', | ||
'file_url': 'ftp://{}/{}'.format(self.ftp_domain, fq_filename), | ||
} | ||
|
||
ret1 = self.impl.download_web_file(self.ctx, params)[0] | ||
|
@@ -1643,18 +1658,16 @@ def test_download_ftp_link_archive_file(self): | |
|
||
fq_filename = "zip1.zip" | ||
|
||
with ftplib.FTP('ftp.uconn.edu') as ftp_connection: | ||
with ftplib.FTP(self.ftp_domain) as ftp_connection: | ||
ftp_connection.login('anonymous', '[email protected]') | ||
ftp_connection.cwd("/48_hour/") | ||
|
||
if fq_filename not in ftp_connection.nlst(): | ||
fh = open(os.path.join("data", fq_filename), 'rb') | ||
ftp_connection.storbinary('STOR zip1.zip', fh) | ||
fh.close() | ||
with open(os.path.join("data", fq_filename), 'rb') as fh: | ||
ftp_connection.storbinary('STOR {}'.format(fq_filename), fh) | ||
|
||
params = { | ||
'download_type': 'FTP', | ||
'file_url': 'ftp://ftp.uconn.edu/48_hour/zip1.zip', | ||
'file_url': 'ftp://{}/{}'.format(self.ftp_domain, fq_filename), | ||
} | ||
|
||
ret1 = self.impl.download_web_file(self.ctx, params)[0] | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Looks like @jsfillman, @Tianhao-Gu and @qzzhang added code without release notes for 1.2 and 1.3 - can the 3 of you update the release notes please?
We should probably add a checklist or something, as I merged a PR without release notes myself. Should've caught that.
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.
👍 updated release note for 0.1.2.
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.
Thanks @Tianhao-Gu . I increased the version to v0.1.3 because there was a new commit with respect to the v0.1.2 that's released on CI (and it blocks re-release of the same version). That implies there was another change after the first v0.1.2. Anyone know what that change was? Maybe I can figure it out from the commit logs, but I figure you all might know rather than me resort to code forensics.
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.
actually, it wasn't too bad. The released commit on CI is df0bb65 which was from @qzzhang. Ever commit after than is from @jsfillman and appears to be his cleanup of our github framework. Not sure what that release note should be. @jsfillman what should we say is the change in RELEASE_NOTES.md for v0.1.3?
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.
@qzzhang do you mind adding some release notes for #67 ?
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.
@MrCreosote @Tianhao-Gu @jsfillman @qzzhang where are we at with this? are the only changes still to record in the release notes the github stuff from @jsfillman and the colon to underscore for SHOCK names?
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.
@MrCreosote I went ahead and added what I could for the comments. I'd really like to merge this so I can register it on CI.KBASE.US and do integration testing of client modules against it. I'm sure @jsfillman would be better able to describe what changes he made between v0.1.2 and v0.1.3 (which I'm considering everything after commit df0bb65), but I made a much more general comment so we can get this through. What do you think?
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.
I updated the release note for #67, then it created a new branch and pr #73.
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.
Also added release notes as requested in #74.
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.
merged your RELEASE_NOTES changes. Working on errors in unit tests now.