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

Package contents cannot have a node named 'file' #314

Closed
evamaxfield opened this issue Jan 26, 2018 · 7 comments
Closed

Package contents cannot have a node named 'file' #314

evamaxfield opened this issue Jan 26, 2018 · 7 comments

Comments

@evamaxfield
Copy link
Collaborator

Attempting to push a package in python on 2.8.4 fails with traceback:

Inferring 'transform: id' for README.md
Copying /home/jovyan/data/README.md...
Traceback (most recent call last):
  File "push_package.py", line 3, in <module>
    quilt.build('aicsjackson/aics_11_test', '/home/jovyan/data/build.yml')
  File "/opt/conda/lib/python3.6/site-packages/quilt/tools/command.py", line 425, in build
    _build_internal(package, path, dry_run, env)
  File "/opt/conda/lib/python3.6/site-packages/quilt/tools/command.py", line 450, in _build_internal
    build_from_path(package, path, dry_run=dry_run, env=env)
  File "/opt/conda/lib/python3.6/site-packages/quilt/tools/command.py", line 522, in build_from_path
    build_package(owner, pkg, path, dry_run=dry_run, env=env)
  File "/opt/conda/lib/python3.6/site-packages/quilt/tools/build.py", line 319, in build_package
    checks_contents=checks_contents, dry_run=dry_run, env=env)
  File "/opt/conda/lib/python3.6/site-packages/quilt/tools/build.py", line 345, in build_package_from_contents
    checks_contents=checks_contents, dry_run=dry_run, env=env)
  File "/opt/conda/lib/python3.6/site-packages/quilt/tools/build.py", line 110, in _build_node
    checks_contents=checks_contents, dry_run=dry_run, env=env, ancestor_args=group_args)
  File "/opt/conda/lib/python3.6/site-packages/quilt/tools/build.py", line 110, in _build_node
    checks_contents=checks_contents, dry_run=dry_run, env=env, ancestor_args=group_args)
  File "/opt/conda/lib/python3.6/site-packages/quilt/tools/build.py", line 121, in _build_node
    path = os.path.join(build_dir, rel_path)
  File "/opt/conda/lib/python3.6/posixpath.py", line 92, in join
    genericpath._check_arg_types('join', a, *p)
  File "/opt/conda/lib/python3.6/genericpath.py", line 149, in _check_arg_types
    (funcname, s.__class__.__name__)) from None
TypeError: join() argument must be str or bytes, not 'dict'

Code:

import quilt

quilt.build('aicsjackson/aics_11_test', '/home/jovyan/data/build.yml')
quilt.push('aicsjackson/aics_11_test', public=True)

Python: 3.6.3
OS: Dockerized Linux (base jupyter/minimal-notebook)

Note: Without README.md file it still terminates the package push.

Package Build.yml:

contents:
  README:
    file: README.md
  data:
    modified_AICS_11_0:
      basic_info:
        file: data/modified-AICS-11-0/basic_info.json
      file:
        file: data/modified-AICS-11-0/file.tif
    modified_AICS_11_1:
      basic_info:
        file: data/modified-AICS-11-1/basic_info.json
      file:
        file: data/modified-AICS-11-1/file.tif
    modified_AICS_11_10:
      basic_info:
        file: data/modified-AICS-11-10/basic_info.json
      file:
        file: data/modified-AICS-11-10/file.tif
    modified_AICS_11_11:
      basic_info:
        file: data/modified-AICS-11-11/basic_info.json
      file:
        file: data/modified-AICS-11-11/file.tif
    modified_AICS_11_12:
      basic_info:
        file: data/modified-AICS-11-12/basic_info.json
      file:
        file: data/modified-AICS-11-12/file.tif
    modified_AICS_11_13:
      basic_info:
        file: data/modified-AICS-11-13/basic_info.json
      file:
        file: data/modified-AICS-11-13/file.tif
    modified_AICS_11_14:
      basic_info:
        file: data/modified-AICS-11-14/basic_info.json
      file:
        file: data/modified-AICS-11-14/file.tif
    modified_AICS_11_15:
      basic_info:
        file: data/modified-AICS-11-15/basic_info.json
      file:
        file: data/modified-AICS-11-15/file.tif
    modified_AICS_11_16:
      basic_info:
        file: data/modified-AICS-11-16/basic_info.json
      file:
        file: data/modified-AICS-11-16/file.tif
    modified_AICS_11_17:
      basic_info:
        file: data/modified-AICS-11-17/basic_info.json
      file:
        file: data/modified-AICS-11-17/file.tif
    modified_AICS_11_18:
      basic_info:
        file: data/modified-AICS-11-18/basic_info.json
      file:
        file: data/modified-AICS-11-18/file.tif
    modified_AICS_11_19:
      basic_info:
        file: data/modified-AICS-11-19/basic_info.json
      file:
        file: data/modified-AICS-11-19/file.tif
    modified_AICS_11_2:
      basic_info:
        file: data/modified-AICS-11-2/basic_info.json
      file:
        file: data/modified-AICS-11-2/file.tif
    modified_AICS_11_20:
      basic_info:
        file: data/modified-AICS-11-20/basic_info.json
      file:
        file: data/modified-AICS-11-20/file.tif
    modified_AICS_11_21:
      basic_info:
        file: data/modified-AICS-11-21/basic_info.json
      file:
        file: data/modified-AICS-11-21/file.tif
    modified_AICS_11_22:
      basic_info:
        file: data/modified-AICS-11-22/basic_info.json
      file:
        file: data/modified-AICS-11-22/file.tif
    modified_AICS_11_23:
      basic_info:
        file: data/modified-AICS-11-23/basic_info.json
      file:
        file: data/modified-AICS-11-23/file.tif
    modified_AICS_11_24:
      basic_info:
        file: data/modified-AICS-11-24/basic_info.json
      file:
        file: data/modified-AICS-11-24/file.tif
    modified_AICS_11_25:
      basic_info:
        file: data/modified-AICS-11-25/basic_info.json
      file:
        file: data/modified-AICS-11-25/file.tif
    modified_AICS_11_26:
      basic_info:
        file: data/modified-AICS-11-26/basic_info.json
      file:
        file: data/modified-AICS-11-26/file.tif
    modified_AICS_11_27:
      basic_info:
        file: data/modified-AICS-11-27/basic_info.json
      file:
        file: data/modified-AICS-11-27/file.tif
    modified_AICS_11_28:
      basic_info:
        file: data/modified-AICS-11-28/basic_info.json
      file:
        file: data/modified-AICS-11-28/file.tif
    modified_AICS_11_29:
      basic_info:
        file: data/modified-AICS-11-29/basic_info.json
      file:
        file: data/modified-AICS-11-29/file.tif
    modified_AICS_11_3:
      basic_info:
        file: data/modified-AICS-11-3/basic_info.json
      file:
        file: data/modified-AICS-11-3/file.tif
    modified_AICS_11_4:
      basic_info:
        file: data/modified-AICS-11-4/basic_info.json
      file:
        file: data/modified-AICS-11-4/file.tif
    modified_AICS_11_5:
      basic_info:
        file: data/modified-AICS-11-5/basic_info.json
      file:
        file: data/modified-AICS-11-5/file.tif
    modified_AICS_11_6:
      basic_info:
        file: data/modified-AICS-11-6/basic_info.json
      file:
        file: data/modified-AICS-11-6/file.tif
    modified_AICS_11_7:
      basic_info:
        file: data/modified-AICS-11-7/basic_info.json
      file:
        file: data/modified-AICS-11-7/file.tif
    modified_AICS_11_8:
      basic_info:
        file: data/modified-AICS-11-8/basic_info.json
      file:
        file: data/modified-AICS-11-8/file.tif
    modified_AICS_11_9:
      basic_info:
        file: data/modified-AICS-11-9/basic_info.json
      file:
        file: data/modified-AICS-11-9/file.tif

Any help and info would be appreciated.

@evamaxfield
Copy link
Collaborator Author

evamaxfield commented Jan 26, 2018

Resolved:

Individual files cannot be named file.{extenstion}.
In my case, file.tif was not a valid path for contents of package.

@dimaryaz
Copy link
Contributor

Good catch! Actually, that is a real bug - you should be free to use the name file. We'll fix it.

@dimaryaz dimaryaz reopened this Jan 26, 2018
@evamaxfield evamaxfield changed the title Issue with Python push package on 2.8.4 Package contents must not have files named file.{extenstion} Jan 26, 2018
@evamaxfield
Copy link
Collaborator Author

@dimaryaz Renamed issue for better tracking. Good luck!

@eode eode changed the title Package contents must not have files named file.{extenstion} Package contents cannot have a node named 'file' Mar 2, 2018
@eode
Copy link
Contributor

eode commented Mar 2, 2018

Actually, this isn't a bug. Perhaps the design needs changing, but it's by design.

It's not a file named 'file.xyz' that is the problem, it's having a node named 'file'. Also, the error occurs during build, not push. That error is really a beast, though, and it's very obscure to users what is actually happening. At the very least, we should change the error message, and perhaps we should change the quilt build file format.

So, for example:

# this package fails with the reported (obscure and unhelpful) error
contents:
  file:
    file:  baz.tif
# this package succeeds
contents:
  file_:        # note the underscore
    file: file.tif

This is because file is a quilt keyword used to specify that the containing node references a file. So, you can reference a file named file, or named file.file, but you can't have a node named file.

I'm going to have a conversation with some others about this, and do some checks in the code to ensure that the issue doesn't cause problems elsewhere. As long as that comes up clean, we could potentially be able to determine that it's being used as a keyword or not by the data it contains.

@evamaxfield
Copy link
Collaborator Author

Yep, I realized the issue a little while ago. Definitely a mistake on my part not following build manifest spec.

@eode
Copy link
Contributor

eode commented Mar 6, 2018

True, but we've decided to allow this behaviour, so in the future you should be able to have packages which use keywords as node names, at least to some extent (including the case you have here).

dimaryaz pushed a commit that referenced this issue Mar 7, 2018
- Handle files named 'file.*'
- Support arbitrary node names, including reserved ones, e.g. file, kwargs etc.

Currently, node parser fails in case there is a file called 'file.*'. I believe it's related to the #314 issue.
@akarve
Copy link
Member

akarve commented Mar 14, 2018

Partially tackled in #414 and will improve with planned syntax checking for build.yml files

@akarve akarve closed this as completed Mar 14, 2018
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

No branches or pull requests

4 participants