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

Cleanup task not keeping releases correctly #1175

Closed
mikeymike opened this issue Apr 18, 2017 · 6 comments
Closed

Cleanup task not keeping releases correctly #1175

mikeymike opened this issue Apr 18, 2017 · 6 comments
Labels

Comments

@mikeymike
Copy link
Contributor

Q A
Issue Type Bug
Deployer Version master
Local Machine OS N/A
Remote Machine OS N/A

Description

Example of the bug can be seen on https://3v4l.org/cXYMq

When you only want to keep 1 release it deletes it, whilst keeping more releases it seems to keep 1 less than required

@mikeymike
Copy link
Contributor Author

As mentioned in the PR #1176 my use case separates code deployment and release symlink into two separate commands because our release could be a DB upgrade release or may just be an atomic release, so a deploy may look something like below

vendor/bin/dep deploy production
vendor/bin/dep release:upgrade production

With this in mind the second command includes the new release in the release list while common deploys with one command will use the cache from a get('release_list') call from earlier in the task list.

In my opinion this caching is causing unexpected behaviour ... what do you think ?

@antonmedv
Copy link
Member

I agree. I think we need to update releases list cache after new release and use your pr.

@mikeymike
Copy link
Contributor Author

What are your thoughts on defining if it's cachable on the set method .. so it's signature would change to set(name, value, cacheable = true)

This way there are things that never change during deployment that can be cached, e.g. binary locations etc while other closures can be set to run each time they are called by setting cacheable to false

@antonmedv
Copy link
Member

No, i think it will make stuff more complicated. I just was thinking about adding this to deployer:release:

add('releases_list', $new_release);

@mikeymike
Copy link
Contributor Author

Ahh yeah that makes sense, I'm still learning the codebase so was unaware of that function 😄 , I'll update and reopen that PR with this change later today 👍

@antonmedv
Copy link
Member

Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants