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

Remove the declaration of thrown runtime exceptions across codebase. #471

Closed
rohanKanojia opened this issue Nov 3, 2020 · 2 comments · Fixed by #627
Closed

Remove the declaration of thrown runtime exceptions across codebase. #471

rohanKanojia opened this issue Nov 3, 2020 · 2 comments · Fixed by #627
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@rohanKanojia
Copy link
Member

Description

There are a few places in our codebase where we have throws XYZException in our methods, even if XYZException is a RuntimeException. We should refactor these places to remove these throw occurrences. Consider this place:
https://github.com/eclipse/jkube/blob/90439cb98fef038e8c991825d2a90ce414787fb6/jkube-kit/build/service/docker/src/main/java/org/eclipse/jkube/kit/build/service/docker/access/ContainerHostConfig.java#L95-L98

This should actually look like this:

public ContainerHostConfig extraHosts(List<String> extraHosts) { 
     // ...
}

You can find out these places via a simple grep command:

jkube : $ git grep -in "throws KubernetesClientException\|throws IllegalArgumentException\|throws NumberFormatException\|throws ExternalConfigHandlerException"
jkube-kit/build/service/docker/src/main/java/org/eclipse/jkube/kit/build/service/docker/access/ContainerCreateConfig.java:75:    public ContainerCreateConfig environment(String envPropsFile, Map<String, String> env, Map mavenProps) throws IllegalArgumentException {
jkube-kit/build/service/docker/src/main/java/org/eclipse/jkube/kit/build/service/docker/access/ContainerHostConfig.java:95:    public ContainerHostConfig extraHosts(List<String> extraHosts) throws IllegalArgumentException {
jkube-kit/build/service/docker/src/main/java/org/eclipse/jkube/kit/build/service/docker/access/PortMapping.java:85:     * @throws IllegalArgumentException if the format doesn't fit
jkube-kit/build/service/docker/src/main/java/org/eclipse/jkube/kit/build/service/docker/access/PortMapping.java:261:    private String createPortSpec(String port, String protocol) throws NumberFormatException {
jkube-kit/build/service/docker/src/main/java/org/eclipse/jkube/kit/build/service/docker/access/PortMapping.java:341:    private void parsePortMapping(String input) throws IllegalArgumentException {
jkube-kit/build/service/docker/src/main/java/org/eclipse/jkube/kit/build/service/docker/config/handler/ExternalConfigHandler.java:45:     * @throws ExternalConfigHandlerException if there is a problem resolving the image configuration
jkube-kit/build/service/docker/src/main/java/org/eclipse/jkube/kit/build/service/docker/config/handler/ExternalConfigHandler.java:48:        throws ExternalConfigHandlerException;
jkube-kit/build/service/docker/src/main/java/org/eclipse/jkube/kit/build/service/docker/config/handler/ImageConfigResolver.java:69:     * @throws IllegalArgumentException if no type is given when an external reference configuration is provided
jkube-kit/build/service/docker/src/main/java/org/eclipse/jkube/kit/build/service/docker/config/handler/compose/ComposeUtils.java:68:     * @throws IllegalArgumentException if {@code pathToResolve} is relative, and {@code project} is {@code null} or
jkube-kit/build/service/docker/src/main/java/org/eclipse/jkube/kit/build/service/docker/config/handler/property/PropertyMode.java:36:     * @throws IllegalArgumentException if empty or invalid names
jkube-kit/build/service/docker/src/main/java/org/eclipse/jkube/kit/build/service/docker/helper/DockerPathUtil.java:43:     * @throws IllegalArgumentException if the supplied {@code baseDir} does not represent an absolute path
jkube-kit/build/service/docker/src/main/java/org/eclipse/jkube/kit/build/service/docker/helper/VolumeBindingUtil.java:158:     * @throws IllegalArgumentException if the supplied {@code baseDir} is not absolute
jkube-kit/build/service/docker/src/main/java/org/eclipse/jkube/kit/build/service/docker/helper/VolumeBindingUtil.java:220:     * @throws IllegalArgumentException if the supplied {@code baseDir} is not absolute
jkube-kit/common/src/main/java/org/eclipse/jkube/kit/common/util/KubernetesHelper.java:117:     * @throws IllegalArgumentException exception if arguement is invalid
jkube-kit/common/src/main/java/org/eclipse/jkube/kit/common/util/KubernetesHelper.java:119:    public static String validateKubernetesId(String currentValue, String description) throws IllegalArgumentException {
jkube-kit/config/image/src/main/java/org/eclipse/jkube/kit/config/image/ImageName.java:232:     * d@throws IllegalArgumentException if the name doesnt validate
jkube-kit/config/image/src/main/java/org/eclipse/jkube/kit/config/image/build/DockerFileBuilder.java:109:     * @throws IllegalArgumentException if no src/dest entries have been added
jkube-kit/config/resource/src/main/java/org/eclipse/jkube/kit/config/resource/ProcessorConfig.java:120:     * @throws IllegalArgumentException if the includes reference an non existing element
jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/ApplyService.java:1225:    public void doCreateJob(Job job, String namespace, String sourceName) throws KubernetesClientException {

For more information, you can check this sonar rule[0]

[0] https://sonarcloud.io/organizations/jkubeio/rules?open=java%3AS1130&rule_key=java%3AS1130

@manusa
Copy link
Member

manusa commented Jan 18, 2021

Declarations have been removed from method signatures, but still remain as JavaDoc entries:

Targets
    Occurrences of 'throws Illeg' in Directory /home/user/00-MN/projects/forks/jkube
jkube-kit-build-service-docker  (7 usages found)
    org.eclipse.jkube.kit.build.service.docker.access  (1 usage found)
        PortMapping.java  (1 usage found)
            85 * @throws IllegalArgumentException if the format doesn't fit
    org.eclipse.jkube.kit.build.service.docker.config.handler  (1 usage found)
        ImageConfigResolver.java  (1 usage found)
            69 * @throws IllegalArgumentException if no type is given when an external reference configuration is provided
    org.eclipse.jkube.kit.build.service.docker.config.handler.compose  (1 usage found)
        ComposeUtils.java  (1 usage found)
            68 * @throws IllegalArgumentException if {@code pathToResolve} is relative, and {@code project} is {@code null} or
    org.eclipse.jkube.kit.build.service.docker.config.handler.property  (1 usage found)
        PropertyMode.java  (1 usage found)
            36 * @throws IllegalArgumentException if empty or invalid names
    org.eclipse.jkube.kit.build.service.docker.helper  (3 usages found)
        DockerPathUtil.java  (1 usage found)
            43 * @throws IllegalArgumentException if the supplied {@code baseDir} does not represent an absolute path
        VolumeBindingUtil.java  (2 usages found)
            158 * @throws IllegalArgumentException if the supplied {@code baseDir} is not absolute
            220 * @throws IllegalArgumentException if the supplied {@code baseDir} is not absolute
jkube-kit-config-image  (2 usages found)
    org.eclipse.jkube.kit.config.image  (1 usage found)
        ImageName.java  (1 usage found)
            232 * d@throws IllegalArgumentException if the name doesnt validate
    org.eclipse.jkube.kit.config.image.build  (1 usage found)
        DockerFileBuilder.java  (1 usage found)
            109 * @throws IllegalArgumentException if no src/dest entries have been added
jkube-kit-config-resource  (1 usage found)
    org.eclipse.jkube.kit.config.resource  (1 usage found)
        ProcessorConfig.java  (1 usage found)
            120 * @throws IllegalArgumentException if the includes reference an non existing element

@manusa manusa added good first issue Good for newcomers help wanted Extra attention is needed and removed good first issue Good for newcomers help wanted Extra attention is needed labels Jan 18, 2021
@theexplorist
Copy link
Contributor

Please assign me this issue @rohanKanojia

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants