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

The no config file found message is unclear for new users #2164

Closed
ThomasLaPiana opened this issue Jan 9, 2023 · 8 comments · Fixed by #2266
Closed

The no config file found message is unclear for new users #2164

ThomasLaPiana opened this issue Jan 9, 2023 · 8 comments · Fixed by #2266
Assignees

Comments

@ThomasLaPiana
Copy link
Contributor

ThomasLaPiana commented Jan 9, 2023

Is your feature request related to a specific problem?

The CLI will no longer write out a fides.toml be default whenever it is run, this leads to users seeing the no config file found message every time they run the CLI without a clear idea of why that is the case.

Describe the solution you'd like

We should update the message to include mention of the fides init command that will write out the config file

Describe alternatives you've considered, if any

This would primarily be hit by people running fides deploy for the first time, maybe we could make it a default part of the fides deploy up codepath?

Additional context

This was introduced with Fides 2.4.0

EDIT January 14, 2023: We've decided that the execution plan is to remove the warning completely. More details in the comments

@rsilvery
Copy link
Contributor

rsilvery commented Jan 9, 2023

@NevilleS , @ThomasLaPiana can we switch back to the more user-friendly method of defaulting to generating this and providing a helpful error if they don't have write permissions?

@NevilleS
Copy link
Contributor

NevilleS commented Jan 9, 2023

Here's another option to consider: we don't need to "warn" the user at all, we can just remove the message.

With few exceptions, are we at the point now where most CLI commands can be run without a config file entirely? Exceptions would include fides webserver which does have some mandatory settings that we can't provide defaults for, but otherwise I feel the message itself may be completely unnecessary...

@NevilleS
Copy link
Contributor

NevilleS commented Jan 9, 2023

To summarize my understanding: the No config file found message appears on every CLI invocation, which we can solve by:

  1. Switching back to the behaviour to create a config file by default
  2. Changing the message to be more helpful (e.g. "Using default configuration values (run 'fides init' to create a fides.toml to change these!)")
  3. Removing the warning message entirely (ie. silently use default configuration values)

Is that about right?

@rsilvery
Copy link
Contributor

rsilvery commented Jan 9, 2023

What are the CLI commands (or UI components) that can't be run without this file? Maybe it makes sense to generate these when those are used?

@NevilleS
Copy link
Contributor

NevilleS commented Jan 9, 2023

Actually I wasn't super precise about that: the file itself should never be required, even for fides webserver which (should be) the only command that needs config settings to run. You can provide those settings via environment variables instead.

I'd very much like to keep the behaviour of not writing the file out as a side-effect of running CLI commands, because it's behaviour that has caused some deployment pains a few times for us... so if option 2 or 3 is doable, that'd be my preference!

What do we think: is it reasonable to just remove the warning entirely? Do we really need to guide users to create a file if it's unnecessary for most usage?

@rsilvery
Copy link
Contributor

rsilvery commented Jan 9, 2023

I feel like I'm still missing something. Are there any cases where not having this file would block a user from doing anything in the UI or CLI? If not let's fix the right problem!

@ThomasLaPiana
Copy link
Contributor Author

@rsilvery Nope, this file is never required. There are certain configuration values that are required to run the webserver, but those can be set via environment variables.

The simplest solution to me seems like removing that "warning" altogether. If a user tries to run the webserver without the necessary values we handle that separately and let the user know.

Currently I am not sure if the "warning" is actually serving much of a purpose other than letting the user know that they could be using a configuration file.

@rsilvery
Copy link
Contributor

Yes, this makes sense to me. Thank you both for patiently explaining :)

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

Successfully merging a pull request may close this issue.

3 participants