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

[fs-irods#18] when closing data objects at process exit, close iRODSFS handles first #611

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

d-w-moore
Copy link
Collaborator

No description provided.

@trel
Copy link
Member

trel commented Aug 23, 2024

why do we need this here? importing a project that depends on this one?

@d-w-moore
Copy link
Collaborator Author

why do we need this here? importing a project that depends on this one?

If the iRODSFS class is successfully loaded into the interpreter, it means there can be file handles needing to be closed.

@trel
Copy link
Member

trel commented Aug 23, 2024

Shouldn't... they worry about that? Can discush.

@d-w-moore
Copy link
Collaborator Author

Shouldn't... they worry about that? Can discush.

Yes they should, in an ideal design.

@d-w-moore d-w-moore marked this pull request as draft August 23, 2024 12:42
@d-w-moore
Copy link
Collaborator Author

d-w-moore commented Aug 23, 2024

I wouldn't normally refer to higher abstractions within a lower-level code base, but this one might be special. iRODSFS library has very good potential for making high level file objects with all the nice abstractions one is used to seeing in I/O libraries - such as a choice of Unicode encodings, for example.

fs = iRODSFS(session)
file_object =  fs.open('/path/to/wide_char_output', 'w', encoding='utf-16')

I admit this code change to be introducing an ugly reverse dependency, inserted in a surprising way, so I've made it a draft PR now. In its defense though, there is a built-in fallback in case the other library isn't there.

Potentially there's a way to make this into a more generic interface, the way we did for progress bars. I'll have to give that some thought.

@d-w-moore
Copy link
Collaborator Author

d-w-moore commented Aug 23, 2024

I haven't heard from @hechth in a while, so I'm wondering if we should not just make our own fork for internal use - or, just bring it in as part of PRC. Maybe fs-irods would then get imported as irods.fs

@trel
Copy link
Member

trel commented Aug 23, 2024

interesting. his project is MIT Licensed... so we'd need to work that out...

@korydraughn
Copy link
Contributor

@d-w-moore Let's discush this.

@d-w-moore
Copy link
Collaborator Author

So any use of or reference to it is protected in some way?... hmm. But he did mention at some point it would be desirable to turn it over to us.

@d-w-moore
Copy link
Collaborator Author

@d-w-moore Let's discush this.

Certainly. We've got time too, there is no rush on this.

@d-w-moore
Copy link
Collaborator Author

d-w-moore commented Aug 25, 2024

interesting. his project is MIT Licensed... so we'd need to work that out...

Ok, we wouldn't have to make it part of the PRC - probably not a good idea anyway. Theoretically we wouldn't have to adopt the MIT license if we treated fs_irods.iRODSFS as an module or plugin to be imported by us, and push all the iRODSFS-related cleanup code to be part of an optional finalize( ) method inside of that module.

@trel
Copy link
Member

trel commented Aug 25, 2024

correct, separate modules can be licensed independently (as they are right now).

right, all cleanup should/must happen on the importer's side, not ours.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants