-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Shared State #2858
Shared State #2858
Conversation
Sources/ComposableArchitecture/Documentation.docc/Articles/SharingState.md
Outdated
Show resolved
Hide resolved
fileExists: { FileManager.default.fileExists(atPath: $0.path) }, | ||
fileSystemSource: { | ||
let source = DispatchSource.makeFileSystemObjectSource( | ||
fileDescriptor: open($0.path, O_EVTONLY), |
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.
Hi there!
Great work on this amazing feature, @stephencelis and @mbrandonw! 😍
I apologize for commenting on an already closed PR, but I believe this is quite important. According to the documentation:
Upon successful completion, the function shall open the file and return a non-negative integer representing the lowest numbered unused file descriptor. Otherwise, -1 shall be returned and errno set to indicate the error. No files shall be created or modified if the function returns -1.1
It appears the open
function will return -1
if the system lacks access to the specified path. If a user specifies @Shared(.fileStorage)
with an URL
that the system cannot access, then calling DispatchSource.makeFileSystemObjectSource
with a file descriptor represented by -1
will lead to a crash.
Is this the intended behavior? While I understand the desire to inform developers when they are accessing a file to which they do not have rights, some operating systems implement mechanisms to dynamically enable or disable access to files. Suppose the access is disabled after the application starts; this would lead to the application crashing.
Do you think we could handle this situation?
Footnotes
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.
@Matejkob We'd take a PR that bails out on a negative file descriptor if you're down to submit! Since it's generally a developer-specified URL then failure to access will generally be a programmer error, so a crash is appropriate there, but there is the edge case where the URL could come in from the user, so we shouldn't make the assumption.
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.
Sure thing, can do. The question is how do you wanna handle such an error?
The createDirectory
endpoint can throw an error as well and I can see that you handled it by ignoring it:
Line 73 in 8624a8e
try? self.storage.createDirectory(self.url.deletingLastPathComponent(), true) |
Do you think it would be appropriate solution here? Or maybe we should emit error down to the user land?
…ure to from: "1.10.0" (#1052) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [pointfreeco/swift-composable-architecture](https://github.com/pointfreeco/swift-composable-architecture) | minor | `from: "1.9.3"` -> `from: "1.10.0"` | --- ### Release Notes <details> <summary>pointfreeco/swift-composable-architecture (pointfreeco/swift-composable-architecture)</summary> ### [`v1.10.0`](https://github.com/pointfreeco/swift-composable-architecture/releases/tag/1.10.0) [Compare Source](https://github.com/pointfreeco/swift-composable-architecture/compare/1.9.3...1.10.0) #### What's Changed - Added: All new state sharing tools, including the `@Shared` property wrapper and more ([https://github.com/pointfreeco/swift-composable-architecture/pull/2858](https://github.com/pointfreeco/swift-composable-architecture/pull/2858)). (Thanks [@​NFulkerson](https://github.com/NFulkerson), [@​hallee](https://github.com/hallee), [@​pyrtsa](https://github.com/pyrtsa), [@​DandyLyons](https://github.com/DandyLyons), [@​hiltonc](https://github.com/hiltonc), [@​lukeredpath](https://github.com/lukeredpath), [@​andtie](https://github.com/andtie), [@​AlexKobachiJP](https://github.com/AlexKobachiJP), [@​ZevEisenberg](https://github.com/ZevEisenberg), for their contributions!) - Updated: Bumped swift-collections to remove `@unchecked` from `StackState`'s `Sendable` conformance (thanks [@​rhysm94](https://github.com/rhysm94), [https://github.com/pointfreeco/swift-composable-architecture/pull/2997](https://github.com/pointfreeco/swift-composable-architecture/pull/2997)). - Updated: Bumped swift-custom-dump to avoid a crash and leverage the latest tools for shared state (thanks [@​y-mimura](https://github.com/y-mimura), [https://github.com/pointfreeco/swift-composable-architecture/pull/3008](https://github.com/pointfreeco/swift-composable-architecture/pull/3008)). - Infrastructure: Update README.md package addition wording to match latest Xcode (thanks [@​dafurman](https://github.com/dafurman), [https://github.com/pointfreeco/swift-composable-architecture/pull/2998](https://github.com/pointfreeco/swift-composable-architecture/pull/2998)). - Infrastructure: Tutorial fixes and updates (thanks [@​dafurman](https://github.com/dafurman), [https://github.com/pointfreeco/swift-composable-architecture/pull/3003](https://github.com/pointfreeco/swift-composable-architecture/pull/3003)). #### New Contributors - [@​dafurman](https://github.com/dafurman) made their first contribution in [https://github.com/pointfreeco/swift-composable-architecture/pull/2998](https://github.com/pointfreeco/swift-composable-architecture/pull/2998) - [@​y-mimura](https://github.com/y-mimura) made their first contribution in [https://github.com/pointfreeco/swift-composable-architecture/pull/3008](https://github.com/pointfreeco/swift-composable-architecture/pull/3008) **Full Changelog**: pointfreeco/swift-composable-architecture@1.9.3...1.10.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4xMDkuNCIsInVwZGF0ZWRJblZlciI6IjM2LjEwOS40IiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> Co-authored-by: Self-hosted Renovate Bot <361546+cgrindel-self-hosted-renovate[bot]@users.noreply.github.enterprise.com>
This PR introduces all new tools for sharing state in the Composable Architecture.
For more information and for questions and comments, please see this discussion: #2857