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

Raise error if waved cannot find public/private dir #2130

Closed
mturoci opened this issue Sep 7, 2023 · 7 comments · Fixed by #2378 · May be fixed by #2213
Closed

Raise error if waved cannot find public/private dir #2130

mturoci opened this issue Sep 7, 2023 · 7 comments · Fixed by #2378 · May be fixed by #2213
Labels
feature Feature request good first issue Contributions welcome! server Related to server

Comments

@mturoci
Copy link
Collaborator

mturoci commented Sep 7, 2023

Is your feature request related to a problem? Please describe

Debugging public/private dir configuration is painful currently, because if the path is not correct, the operation fails silently and waved starts nonetheless.

Describe the solution you'd like

Notify the user about the inaccessible path during waved startup.

@mturoci mturoci added server Related to server feature Feature request labels Sep 7, 2023
@mturoci mturoci added the good first issue Contributions welcome! label Oct 10, 2023
@kaitlyncliu
Copy link

Hi I would like to work on this!

@mturoci
Copy link
Collaborator Author

mturoci commented Nov 28, 2023

Go ahead and good luck then @kaitlyncliu!

@kaitlyncliu
Copy link

Hi, just wanted to update that I am working with @92ammarraza on this issue for a class project so he is making the PR!

@chickenleaf
Copy link

@mturoci is this still up for grabs?

@dbranley
Copy link
Contributor

dbranley commented Aug 2, 2024

Hi, I submitted a PR for this change request. Let me know how it looks and if you would like any changes. Thanks!

@pascal-pfeiffer
Copy link

pascal-pfeiffer commented Aug 12, 2024

What is the reasoning process behind disallowing this?
Usually, one would check in the python app code and at startup if the required folder exists and if not it might be created. Especially with mounted devices, this now requires code outside of the app and even prior to startup.

In other words, if this is solely there for dev experience and has no security reasons, I'd suggest to lower it to a warning. In its current state, this is a breaking change for existing pipelines. @mturoci

@mturoci
Copy link
Collaborator Author

mturoci commented Aug 13, 2024

The original goal was to improve DX since specifying folders is a bit inconvenient (has to be relative to waved binary rather than main app file). This way, wrong path would be reported right away.

However, I didn't consider the usecase you mentioned above, which is totally valid, so lowering to warn makes sense.

Changed in 6ebfbf9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature request good first issue Contributions welcome! server Related to server
Projects
None yet
5 participants