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

"ddev list" should list sites based on container even if directory removed #374

Closed
rfay opened this issue Jul 5, 2017 · 9 comments
Closed
Milestone

Comments

@rfay
Copy link
Member

rfay commented Jul 5, 2017

What happened (or feature request):

Since I've stumbled on this a number of times it's likely that users will also stumble. In tests, the site directory has been removed, but the containers remain (poor things). ddev list doesn't show them in this case though. It should probably show them and say "Ooops, dir removed"

What you expected to happen:

I'd expect to see sites (with a warning about them being broken) if their dir had been removed without them being removed.

Version: Please include the output of ddev version and the project's .ddev/config.yaml.

How to reproduce this:

  • Start a site
  • rm -r the site directory
  • ddev list

Anything else do we need to know:

Related source links or issues:

@rfay
Copy link
Member Author

rfay commented Jul 15, 2017

Ran into this again today, think it's worth solving (and very low-hanging fruit)

@rickmanelius
Copy link
Contributor

Not opposed. Just need to determine what the best course of action would be!

@rfay
Copy link
Member Author

rfay commented Jul 17, 2017

This is not hard - we just have something that ignores the site if it can't find the directory. This is a tiny fix.

@rickmanelius
Copy link
Contributor

@rfay cool! Would we instruct people on how to remove the docker containers if they are orphaned? Or would we provide instructions on how to reconnect?

@rickmanelius
Copy link
Contributor

This was a tactical fail on my part. I should have tested and thought of a more thoughtful response to get to a clear "yes" before just talking off the cuff and making this more noisy than it had to be. I'll test before responding further and I'm sure it'll be the quick and easy fix you're suggesting!

@rfay
Copy link
Member Author

rfay commented Jul 17, 2017

ddev rm removes them just fine; only ddev list has this problem. So if it says in the directory has been removed people will say OH and do the right thing.

1__default__bash__and__ddev_list__should_list_sites_based_on_container_even_if_directory_removed_ issue__374 _drud_ddev

@rickmanelius
Copy link
Contributor

I've had time to review and here's my take:

  • CONFIRMED: I'm able to recreate the issue using the instructions posted in the issue summary.
  • Areas that this affects:
    • ddev list inclusion: the app's docker containers are still running, but ddev list doesn't show them.
    • docker container clean-up: the app's docker containers are still running, but ddev can no longer be used to remove them.
    • ddev list output: technically we can still output the app name, type, and URL. The status would need to be updated to note that the status is "directory removed", but are we able to remember the location?
  • Proposed solution:
    • Switch status to "config deleted".
    • Optional: provide additional info what this means and what to do to clean-up.

@nmccrory
Copy link
Contributor

After reviewing my PR and getting more familiar with this issue, @beeradb and I would like to propose that sites with missing directories get listed in their own section of the ddev list output, coupled with cleanup instructions.

I've spent enough time on this that I can confidently say, distinctly separating active ddev sites from "ghosted" sites - those with missing directories - is the most informative/intuitive to the user.

The proposed solution is simple: if ddev list finds sites with missing root directories, it prints a section to the bottom of the output like so:

The following sites are missing directories:
<site-name>

Tip: clean up sites using ddev rm [site name].

This is a straightforward way to get the user to actually ddev rm the ghosted site.

This change is the latest commit to PR #444 however approval from @rickmanelius is still required.

nmccrory added a commit that referenced this issue Sep 12, 2017
…ved: fix for #374 (#444)

* Add site status for directory removed

* In the case of a missing site directory, initalization can be done manually using .

* Remove erraneous InitFromMissingDirectory function and add some context to our changes using comments.

* Cast site to *LocalApp.

* Handle potential errors while casting using . Use better name for variable storing the casted value.

* Adjusted error message presented on failure to cast.

* Present value of site instead of siteStruct.

* Handle the case of a missing .ddev/config.yml file
@rfay
Copy link
Member Author

rfay commented Sep 26, 2017

This was fixed in #444.

@rfay rfay closed this as completed Sep 26, 2017
@rickmanelius rickmanelius modified the milestones: v1.1.0, v0.9.3 Nov 17, 2017
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

No branches or pull requests

3 participants