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

Add glob 'cleanup_pattern' to remove obsolete resources #253

Merged
merged 2 commits into from
Apr 4, 2021

Conversation

gieza
Copy link

@gieza gieza commented Mar 29, 2021

Hello,

We noticed that over time obsolete resources accumulate in Getdown folder. This PR allows to define optional list of Glob patterns in Getdown.txt, like:

cleanup_pattern = lib\*.jar
cleanup_pattern = rsrcs\*.jpg
cleanup_pattern = old_*.png

Glob patterns are defined relative to 'getdown' folder.
In this case Getdown would collect list for files from the patterns, that is cross-check against resources specified in Getdown.txt. Then only the files that are not in getdown resource list, will be removed.

Potentially closes #183

Giedrius Zavadskis added 2 commits March 29, 2021 10:53
@samskivert
Copy link
Member

This is great thanks! I'll try to review it today or tomorrow and either give feedback or merge it. Thanks again!

@samskivert samskivert merged commit e8db626 into threerings:master Apr 4, 2021
@samskivert
Copy link
Member

Looks great, thanks again!

@gieza
Copy link
Author

gieza commented Apr 6, 2021

Hi,

you are welcome, though there's not much to thank for. We needed ourselves and pull request did not cause a lot of extra work.

I am working on several modifications of Getdown as per list below, that I will push to my fork. If you think those are interesting, I can create pull requests as well.

  • add getdown.txt parameter to disable proxy logic - nowadays proxies are rare and in current Getdown version any network connectivity issues are reported as proxy issues, which gives confusion to the end-user.
  • move failure messages from Status panel to new Failure panel - failure messages are much longer than typical status messages, what makes sizing of status box difficult.
  • make Getdown fully self-updatable - typically initial deployment of Getdown jar and Getdown.txt happen through running an installer. So the solution will be for Getdown to check its own internal version against required in Getdown.txt. If new version is required, then Getdown will download new installer from the server (per link in Getdown.txt as executable resource) and then launch downloaded installer while closing down.
  • enforce always to use custom JVM for the launched app - on one hand since Java9 we have multiple Java vendors, and Java version should be checked also against vendor. On the other hand this allows to disassociate Java version required for Getdown, from version required for the end-user app.
  • make Getdown Java9+ modular and build JLINK minimized image - reduces required disk space for disassociated Java versions for Getdown and then end-user app. Also we do not expect Java to be installed on all machines nowadays

@samskivert
Copy link
Member

That all sounds great. I'm sure everyone will appreciate those improvements.

@serge-xav
Copy link

serge-xav commented Dec 23, 2021

Hi all, unfortunately, with latest commit (332f7f1) , the feature works only for Versioned updates. Unversioned updates will never clean the resource. I'd rather have the cleanup possible for both Versioned and Unversioned, but as soon as there was some update on the getdown.txt file.
What about moving the cleanup code at the end of install() method, if there is updateAvailable ?

@bekoenig
Copy link
Contributor

bekoenig commented Mar 10, 2022

Hej @serge-xav,

you are right. I moved this snip

     /**
     * Installs the currently pending new resources.
     */
    public void install () throws IOException
    {
        if (SysProps.noInstall()) {
            log.info("Skipping install due to 'no_install' sysprop.");
        } else if (isUpdateAvailable()) {
            log.info("Installing " + _toInstallResources.size() + " downloaded resources:");
            for (Resource resource : _toInstallResources) {
                resource.install(true);
            }
            _toInstallResources.clear();
            _readyToInstall = false;
            log.info("Install completed.");

            // do resource cleanup
            final List<String> cleanupPatterns = _app.cleanupPatterns();
            if (!cleanupPatterns.isEmpty()) {
                cleanupResources(cleanupPatterns);
            }
        } else {
            log.info("Nothing to install.");
        }
    }

and I think this should be better!

Greetz,
Ben

@serge-xav
Copy link

Hi, thanks for your commit, I'll test it on my side but it should be OK from my point of view.

BR
Xavier

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 this pull request may close these issues.

Remove unused resources
4 participants