search

ki-tools / kitools-py

Tools for working with data in Ki analyses
Apache License 2.0
3 stars 0 forks source link

Ensure files with invalid characters in the name are handled. #26

Open pcstout opened 5 years ago

pcstout commented 5 years ago

Synapse has restrictions on which characters can be in a file/folder name. Make sure the error message is descriptive enough so the user knows how to fix it.

elgertam commented 5 years ago

Pull request to resolve this issue at https://github.com/ki-tools/kitools-py/pull/35

elgertam commented 5 years ago

One question I have is how we'd like to handle invalid files that are added to the kiproject.json. The exception message informs users that they need to fix the filename and kiproject.json, but I think this could be frustrating for some users. On the other hand, most users probably won't have too many invalid filenames.

hafen commented 5 years ago

@elgertam don't we detect invalid file names before the entry is added to kiproject.json? If that's the case, we can just throw the exception and not write it to kiproject.json.

elgertam commented 5 years ago

@hafen At the moment, we're not checking whether the filename is valid according to Synape's rules; we only check whether it exists locally. Part of the reason for that (as far as I can tell) is that rules for valid filenames on Synapse may not be the same as on other platforms. I could certainly add in the ability to check that a filename is valid based on the project type as specified in the kiproject.json, but it would be a bit more work.

hafen commented 5 years ago

Oh sorry I misunderstood. Where is the filename checking happening in #35?

Are Synapse's rules the lowest common denominator across all platforms?

pcstout commented 5 years ago

@elgertam, @hafen For this issue I was thinking we could delegate the validation (as much as possible) to the providing service (Synapse in this instance). The implementation is probably catching a specific error from Synapse and prettying up the error (if needed at all -- I'm not sure what Synapse spits out but I think it's fairly explanatory).

It will be difficult to catch all invalid characters upon data_add since it can take a remote_uri (i.e., we don't even know the filename or path when adding the resource to the project). It should be as simple as data_remove -> change filename -> data_add for the user to fix filename issues.

hafen commented 5 years ago

I see. That sounds good to me then.