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

Vendoring of UUID generates conflicts #88

Open
thehgregor opened this issue Apr 8, 2020 · 2 comments
Open

Vendoring of UUID generates conflicts #88

thehgregor opened this issue Apr 8, 2020 · 2 comments

Comments

@thehgregor
Copy link

What happened?

When errors reference a primitive of type UUID in an API definition, the code generated by Conjure Go doesn't compile.

API definition:

{
    "version": 1,
    "errors": [
        {
            "errorName": {
                "name": "MyError",
                "package": "uuid.api"
            },
            "docs": "Some error.\n",
            "namespace": "Sample",
            "code": "INVALID_ARGUMENT",
            "safeArgs": [
                {
                    "fieldName": "info",
                    "type": {
                        "type": "reference",
                        "reference": {
                            "name": "InfoType",
                            "package": "uuid.api"
                        }
                    },
                    "docs": null
                }
            ],
            "unsafeArgs": []
        }
    ],
    "types": [
        {
            "type": "object",
            "object": {
                "typeName": {
                    "name": "InfoType",
                    "package": "uuid.api"
                },
                "fields": [
                    {
                        "fieldName": "someField",
                        "type": {
                            "type": "primitive",
                            "primitive": "UUID"
                        },
                        "docs": "The UUID.\n"
                    }
                ],
                "docs": "Some UUID.\n"
            }
        }
    ],
    "services": [
        {
            "serviceName": {
                "name": "MyService",
                "package": "uuid.api.service"
            },
            "endpoints": [
                {
                    "endpointName": "getInfo",
                    "httpMethod": "GET",
                    "httpPath": "/info",
                    "auth": {
                        "type": "header",
                        "header": {}
                    },
                    "args": [],
                    "returns": {
                        "type": "reference",
                        "reference": {
                            "name": "InfoType",
                            "package": "uuid.api"
                        }
                    },
                    "docs": "Some information.\n",
                    "deprecated": null,
                    "markers": []
                }
            ],
            "docs": "Returns information.\n"
        }
    ]
}

Compile error:

# sample/uuid/api
../../../../go/src/sample/uuid/api/errors.conjure.go:77:115: cannot use e.errorInstanceID (type "github.com/palantir/pkg/uuid".UUID) as type "github.com/palantir/conjure-go-runtime/vendor/github.com/palantir/pkg/uuid".UUID in field value
../../../../go/src/sample/uuid/api/errors.conjure.go:89:20: cannot use serializableError.ErrorInstanceID (type "github.com/palantir/conjure-go-runtime/vendor/github.com/palantir/pkg/uuid".UUID) as type "github.com/palantir/pkg/uuid".UUID in assignment

Conjure Go issue palantir/conjure-go#78 might be related to this problem.

What did you want to happen?

Package github.com/palantir/pkg/uuid should not be vendored in conjure-go-runtime or a different strategy to avoid a conflict should be implemented in conjure-go.

@nmiyake
Copy link
Contributor

nmiyake commented Apr 8, 2020

Have you properly run vendoring/module fetching locally? Based on the error message, I think this may be a local build issue (also, there are plenty of definitions that use UUID and properly generate compiling code presently)

@thehgregor
Copy link
Author

For me the issue is that the package's interface is exposing a type from a vendored module. Even if this can be solved otherwise, I would find it cleaner if the module only exposed types from the standard library or types defined within the package itself.

Please correct me if I'm wrong, but for me vendoring has the advantage to protect your package from breaking changes in external dependencies. If a type from such an external dependency is exposed by the package's API, you lose this advantage because you then have to effectively use the same version of the external package both within your package as well as in the program importing it.

In the concrete example the issue is the use of type uuid.UUID in type SerializableError of conjure-go-runtime/conjure-go-contract/errors/serializable_error.go

As UUID is just an alias of [16]byte, I think it would be possible in this case at least to find a different solution for this?

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

2 participants