Skip to content

Conversation

@dimitri-yatsenko
Copy link
Member

@dimitri-yatsenko dimitri-yatsenko commented Sep 14, 2019

@dimitri-yatsenko dimitri-yatsenko marked this pull request as ready for review September 20, 2019 16:07
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 91.475% when pulling 8aa1543 on dimitri-yatsenko:attach into aae57ac on datajoint:dev.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 91.475% when pulling 8aa1543 on dimitri-yatsenko:attach into aae57ac on datajoint:dev.

@coveralls
Copy link

coveralls commented Sep 20, 2019

Coverage Status

Coverage decreased (-0.6%) to 91.438% when pulling 48147f1 on dimitri-yatsenko:attach into ef5543b on datajoint:dev.

@dimitri-yatsenko dimitri-yatsenko mentioned this pull request Sep 20, 2019
@guzman-raphael
Copy link
Collaborator

Can you please remove python 3.4 from .travis.yml if we no longer plan on testing against it?

@guzman-raphael
Copy link
Collaborator

Please merge in the latest from dev to include new testing structure.

relpath = 'one/two/three'
os.makedirs(os.path.join(stage_path, relpath), exist_ok=True)
managed_file = os.path.join(stage_path, relpath, filename)
managed_file = Path(stage_path, relpath, filename)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This test failed on WIN10. For example, the result of managed_file was C:\\Users\\rguzman\\AppData\\Local\\Temp\\tmpb3gvp0ht\\one\two\threepicture.dat.

ext.delete(delete_external_files=True)


def test_filepath_s3():
Copy link
Collaborator

Choose a reason for hiding this comment

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

This test failed on WIN10. For example, the result of managed_file was dj/repo/one\two\three\picture.dat.

Copy link
Collaborator

@guzman-raphael guzman-raphael left a comment

Choose a reason for hiding this comment

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

Updated with errors from running nosetests on WIN10.

ext.upload_filepath(managed_file) # this is fine because the file is the same


def test_duplicate_upload_s3():
Copy link
Collaborator

Choose a reason for hiding this comment

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

This test failed on WIN10. For example, the result of managed_file was dj/repo/one\two\three\plot.dat.

ext.upload_filepath(managed_file) # this should raise exception because the file has changed


def test_duplicate_error_s3():
Copy link
Collaborator

Choose a reason for hiding this comment

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

This test failed on WIN10. For example, the result of managed_file was dj/repo/one\two\three\thesis.dat.

relative_path = 'one/two/three'
os.makedirs(os.path.join(stage_path, relative_path), exist_ok=True)
managed_file = os.path.join(stage_path, relative_path, filename)
managed_file = Path(stage_path, relative_path, 'attachment.dat')
Copy link
Collaborator

Choose a reason for hiding this comment

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

This test failed on WIN10. For example, the result of managed_file was C:\\Users\\rguzman\\AppData\\Local\\Temp\\tmpb3gvp0ht\\one\two\threeattachment.dat.

table.external[store].delete(delete_external_files=True)


def test_filepath_class_again():
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • This test failed on WIN10 at test_filepath_class_again. Error details: datajoint.errors.DataJointError: A different version of 'one\two\three\attachment.dat' has already been placed
  • This test failed on WIN10 at test_filepath_class_s3. For example, the result of managed_file was dj/repo/one\two\three\attachment.dat.
  • This test failed on WIN10 at test_filepath_class_s3_again. For example, the result of managed_file was dj/repo/one\two\three\attachment.dat.

managed_file.parent.mkdir(parents=True, exist_ok=True)
with managed_file.open('wb') as f:
f.write(contents) # same in all files
table.insert1((i, managed_file))
Copy link
Collaborator

Choose a reason for hiding this comment

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

This test failed on WIN10. Error details: datajoint.errors.DuplicateError: ("Duplicate entry '1' for key 'PRIMARY'", 'To ignore duplicate entries in insert, set skip_duplicates=True')

Copy link
Member Author

Choose a reason for hiding this comment

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

this happens sometimes when the previous test run was terminated before cleanup



def test_filepath_cleanup_s3():
"""test deletion of filepath entries from external table """
Copy link
Collaborator

Choose a reason for hiding this comment

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

This test failed on WIN10. For example, the result of managed_file was dj/repo/four\three\one\file.dat.

.travis.yml Outdated
- sudo pip install python-coveralls
- coveralls
- coveralls
>>>>>>> bff58baac8da58b43be95ba1c58174cb7721411f
Copy link
Collaborator

@guzman-raphael guzman-raphael Oct 3, 2019

Choose a reason for hiding this comment

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

This line should be removed... Travis tests will no longer run on this PR until this is addressed.

Copy link
Collaborator

@guzman-raphael guzman-raphael left a comment

Choose a reason for hiding this comment

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

Awesome! Feel much better about this version. :)
Only have some minor suggestions for CHANGELOG.md then I will happily approve and merge.

dimitri-yatsenko and others added 2 commits October 7, 2019 22:03
Co-Authored-By: guzman-raphael <[email protected]>
Co-Authored-By: guzman-raphael <[email protected]>
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.

3 participants