Docker - Load images fix

[lukaz-sampaio]

Docker's images don't load because attribute "VirtualSize" was removed from docker engine.

Removing docker's attribute "VirtualSize" that was removed from docker engine.

[matthiasblaesing]

This will fail. The class org.netbeans.modules.docker.api.DockerImage is part of the API of the module and thus removing getVirtualSize is an incompatbile change. The API is "friend only" (only explicitly listed other modules can access it), but still an API.

From my understanding "Virtual Size" never made sense for images, just for containers, as the image are the readonly portion and only a running container has a writable part.

So wouldn't it make sense to use getOrDefault also for virtual size and use the value from Size as fallback? On my admittingly small setup, this seems to be truthy.

[mbien]

I only took a quick look at this yesterday but the API seemed to be only exported to

netbeans/ide/docker.api/nbproject/project.xml

Lines 130 to 133 in 7216834

	<friend-packages>
	<friend>org.netbeans.modules.docker.ui</friend>
	<package>org.netbeans.modules.docker.api</package>
	</friend-packages>

so its essentially internal since I don't think it can leak out of docker.ui. Not saying that we should remove it but I don't think there is anything preventing removal.

[matthiasblaesing]

For me doing a compatible change is just easier. API breakage should always raise concerns and be treated with respect. It also requires bumping major version of the affected module, updating the using module and raising the specification version of that module. It is just messy.

"friend only" dependencies are APIs being stabilized. If you don't want an API, you can use an implementation dependency in the IDE and be done with it.

[lukaz-sampaio]

@matthiasblaesing, would you suggest another alternative? Taking into account the API break, I even thought about putting zero in the virtual size parameter, but it seems like an improvised solution. I'm here to learn.

[matthiasblaesing]

@lukaz-sampaio the alternative from my perspective would be to keep the attribute and fill it on a best-effort basis. If VirtualSize is not present, report the value of Size for it. I looked at the output of docker image ls --format json and to me the two values are just differently rounded. Something like this:

    @SuppressWarnings("unchecked")
    public List<DockerImage> getImages() {
        try {
            JSONArray value = (JSONArray) doGetRequest("/images/json",
                    Collections.singleton(HttpURLConnection.HTTP_OK));
            List<DockerImage> ret = new ArrayList<>(value.size());
            for (Object o : value) {
                JSONObject json = (JSONObject) o;
                JSONArray repoTags = (JSONArray) json.get("RepoTags");
                String id = (String) json.get("Id");
                long created = (long) json.getOrDefault("Created", 0L);
                long size = (long) json.getOrDefault("Size", 0L);
                long virtualSize = (long) json.getOrDefault("VirtualSize", size);
                ret.add(new DockerImage(instance, repoTags, id, created, size, virtualSize));
            }
            return ret;
        } catch (DockerException ex) {
            LOGGER.log(Level.INFO, null, ex);
        }
        return Collections.emptyList();
    }

Callers now get the closest thing to the right size, without breaking compatibility.

[lukaz-sampaio]

I had read an explanation about the difference between "Size" and "VirtualSize" in the docker repository itself. From the explanation, it seems to me that "Size" and "VirtualSize" are different. However, listing the images using a version of the engine (I tested v1.24 which is in the engine documentation and v1.43 which is the last version where the "VirtualSize" attribute was still there) in which the "VirtualSize" attribute was still there, the value for "Size" and "VirtualSize" are the same. Thanks for your help.

[matthiasblaesing]

Assuming you tested, that this actually works (I don't have a new enough docker system), the change looks sane to me.
Would you please squash the commits into a single one?

[mbien]

we could deprecate DockerImage.getVirtualSize and add a comment that it will return getSize after docker v1.43?

it isn't used anywhere:
https://github.com/search?q=repo%3Aapache%2Fnetbeans%20getVirtualSize&type=code

I would be also ok with removal and spec bump if needed, whatever is easier.

[matthiasblaesing]

we could deprecate DockerImage.getVirtualSize and add a comment that it will return getSize after docker v1.43?

@lukaz-sampaio the suggestion from @mbien sounds good to me. Would you mind adding the deprecation and note when you squash?

[matthiasblaesing]

In addition to the @deprecated javadoc tag please also add the @Deprecated annotation (see: https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/Deprecated.html)

[matthiasblaesing]

Lets get this in. Thank you for your contribution @lukaz-sampaio

[lukaz-sampaio]

@matthiasblaesing and @mbien thank you very much for the support and help.