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

Possibly replace CheckEnvironment with a nimbler set of exceptions and handlers #630

Closed
alexweissman opened this issue Feb 1, 2017 · 10 comments
Labels
architecture Related to the framework architecture developer experience The one building with UF error handling Related to UF's error-handling system. up-for-grabs Not assigned yet
Milestone

Comments

@alexweissman
Copy link
Member

As we discussed in #dev, we should consider replacing the monolithic CheckEnvironment with appropriate exceptions and exception handlers in the portions of the code where they are relevant. For example, the check to see if cache/ is writable could happen the first time something actually tries to write to that directory (so, maybe the locator service could do this check and throw the exception).

This would also allow us to preserve the easy toggling between "dev" mode (sending errors to the response) and "production" mode (sending errors to a log and showing the client a generic message).

@lcharette
Copy link
Member

Can we list on the first post what needs to be done so it's easier to plan / split?

@Silic0nS0ldier
Copy link
Member

Silic0nS0ldier commented Feb 3, 2017

I agree with @lcharette, should make it much easier to keep track of progress on this.

Also sounds like an opportunity to use GitHub tasks.
- [ ] Task name

  • Task name

@alexweissman alexweissman added this to the 4.0 under consideration milestone Mar 1, 2017
@lcharette
Copy link
Member

FYI, while most check will be made by the CLI in 4.1, detecting the presence of apache mod rewrite will still need to be done by the core code since apache is not involved in the CLI, therefore the function used to detect mod_rewrite won't be useful in the cli environment.

@lcharette
Copy link
Member

Same goes for folder permission apparently. Checking them via the CLI will return the permission of the user running the cli command, which might be different from the apache user. And using exec trick or looking at the chmod value doesn't seems reliable enough.

@Silic0nS0ldier
Copy link
Member

Whatever solution is landed on, it needs to support Nginx and IIS as well. Might be an idea to have specialised classes for each web server. Such an approach might make it possible to eliminate the need for checkenvironment, provided the CLI tools are decent for Apache and Nginx. (I know IIS is already fully controllable via CLI).

@alexweissman alexweissman modified the milestones: 4.1.x, 4.2.0 Jun 18, 2017
@alexweissman alexweissman added architecture Related to the framework architecture and removed standards and best practices Coding guidelines V4 labels Jul 14, 2017
@lcharette lcharette modified the milestones: 4.2.0, 4.3.0 Dec 16, 2017
@alexweissman alexweissman added developer experience The one building with UF and removed user experience The one using UF labels Dec 20, 2017
@lcharette
Copy link
Member

Do we still need a single page with all the checks? Or could we move to a one exception at a time solution?

For example, Apache and permission fails. Right now both would be displayed on the same page. Do we need to do that, or could be display Apache exception, then, when it's fixed, display the permission exception?

@Silic0nS0ldier
Copy link
Member

I feel like that Apache check could be converted to a behaviour driven test. Something that would work with more than just Apache.

@lcharette
Copy link
Member

At the same time it would be more useful to tell the (new) dev "you need mod rewrite" than "This don't work. It may be mod_rewrite, or ISS thing, or nginx thing"

@Silic0nS0ldier
Copy link
Member

True, but we can always strike a balance.

@lcharette lcharette added the up-for-grabs Not assigned yet label Jun 27, 2019
@lcharette lcharette modified the milestones: 4.4.0, 4.5.0 Nov 23, 2019
@lcharette lcharette added the error handling Related to UF's error-handling system. label Nov 17, 2021
@lcharette lcharette modified the milestones: 4.x.x, 5.0 Nov 17, 2021
@lcharette
Copy link
Member

CheckEnvrionement is gone in UF5 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
architecture Related to the framework architecture developer experience The one building with UF error handling Related to UF's error-handling system. up-for-grabs Not assigned yet
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

3 participants