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

Correctly parse atlas community version #66

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

borchero
Copy link

@borchero borchero commented Apr 28, 2024

Motivation

The atlas-terraform-provider (v0.8.0) fails with the following error when using the Community version of atlas with it:

╷
│ Error: Check atlas version failure
│ 
│   with provider["registry.terraform.io/ariga/atlas"],
│   on main.tf line 26, in provider "atlas":
│   26: provider "atlas" {
│ 
│ unexpected output format
╵

The implementation of the version check is delegated to the atlas-go-sdk which cannot parse the output of atlas version when using a community version. This PR fixes the issue.

Copy link
Member

@a8m a8m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution, @borchero.

However, the API in this SDK assumes a non-community version in some of its options and commands. I am not sure what the gap is, and currently have no plans to spend time on this compatibility, so I’m going to check this before I merge the PR.

@borchero
Copy link
Author

Thanks for your quick reply @a8m!

Since the non-community version is a strict superset of the community version, I guess it is natural that some functions simply fail when the community version is installed. I would argue that checking for a non-community version becomes the responsibility of the SDK user though.

With that in mind, should we maybe just add a Community flag to the Version struct to allow SDK users to check for it?

I'd appreciate if this could be moved forward in the near future as this would fix the Terraform provider for my use case (and for the atlas binary published to conda-forge, c.f. https://anaconda.org/conda-forge/atlasgo). If you don't have enough capacity to think about this at the moment, I respect that, of course 🙏

@borchero
Copy link
Author

I modified my proposal accordingly, I think this adds some value 😄

@borchero
Copy link
Author

borchero commented Jun 2, 2024

@a8m I just realized that ariga/atlas#2740 made the situation in builds of v0.23.0 even worse. There is now no way to use a community build with the SDK due to the introduction of the word "unofficial" in the version string. As a result, I would really like to move forward with this PR. This would likely entail introducing yet another flag, namely Official bool (or the reverse, i.e. Unofficial bool). It is a little unclear to me why this additional distinction is necessary though, the "community" flag should already imply that the build is "unofficial".


Unfortunately, I have to say that finding out about above's PR left a bit of a bad taste in my mouth. I love atlas's functionality but I do find the timing of the PR a little odd (exactly one day after the conda-forge build was published)... it seems to me as if the goal would be to move all users of atlas to the "official", closed-source build. In case this assessment is accurate, I would appreciate if this intent was communicated accordingly 😅 at the time of writing this, the official docs suggest that the open-source version of atlas should suffice for many users, specifically:

For the basic and common use cases, the open-source version of Atlas is more than enough.

In case the knock-on effects of the above change were, in fact, inadvertent, I will patch the conda-forge build to remove the word "unofficial" from the version string to not break integration with existing tools.


To give some context on why one would want to use a build of the community version in the first place:

  • There are obvious security concerns to use closed-source builds of a tool when interacting with production databases
  • There is no way to use the "latest" official build without introducing strange non-deterministic errors due to the use of canary releases
  • Using plain download URLs doesn't allow for easy integration with established tools (e.g. conda) to lock tools' versions and periodically update those

I hope this provides some insights why, I believe, there should exist an open-source build of atlas that integrates with all atlas-related tools. Personally, I would expect that this would increase adoption of the tool which I would love to see.

@a8m
Copy link
Member

a8m commented Jun 3, 2024

It is a little unclear to me why this additional distinction is necessary though, the "community" flag should already imply that the build is "unofficial".

We also release community builds (Apache2 license) twice a day. The other builds that compile Atlas from source are out of our control, might contain a modified version, and I prefer communicate this correctly to our users (paid and unpaid ones).

Unfortunately, I have to say that finding out about above's PR left a bit of a bad taste in my mouth. I love atlas's functionality but I do find the timing of the PR a little odd (exactly one day after the conda-forge build was published)...

I am not aware of this tool/build-system.

If you are on our Discord server, you'll notice that many of my support there in the last year suggest users to install Atlas correctly (after they hit some limitation) because they install the incorrect ones (mostly via brew/nixos). That's actually even worse when our Enterprise users accidentally install these. Since the external builds (and there are multiple of them out there) use the name atlas, it's important to emphasize that these were not made by the Atlas team - I don't have plans to start reviewing these biuilds one by one.


I would argue that checking for a non-community version becomes the responsibility of the SDK user though.

I just saw this comment. It's easy to say this from the perspective of a user, but in practice, it's different. It's our responsibility to ship correct and stable software, and support our users. My experience is just different from managing two big communities (ent and atlas).

I understand the impact and the limitation it brings, but since our capacity is limited, this is going to be on hold at this stage, as it requires our time to make all flows compatible (and safe to use) with the community builds.

@borchero
Copy link
Author

Thanks for your answer @a8m, I see that you have put in quite some thought into this and I don't expect anything to happen here. I still want to leave some additional comments here.

We also release community builds (Apache2 license) twice a day.

Even with that being the case, they are closed-source builds (afaict they are not built via GitHub Actions or any other public CI system) and, even worse, I still can't use official community builds with this SKD.

I am not aware of this tool/build-system.

conda is a cross-platform package manager with conda-forge hosting almost 25,000 packages. Even if it isn't widely used in the Go ecosystem, it is a popular way of installing single-binary CLI tools.

If you are on our Discord server, you'll notice that many of my support there in the last year suggest users to install Atlas correctly (after they hit some limitation) because they install the incorrect ones (mostly via brew/nixos)

I do understand that this is annoying but the distinction between official and unofficial distributions is also largely unclear in the documentation. Homebrew, in particular, is listed as one of the installation options (cf. https://atlasgo.io/getting-started/). It seems strange to expect users to know to not install atlas via brew then.

It's our responsibility to ship correct and stable software, and support our users.

I'm wondering if you could achieve this by making it abundantly clear in the documentation that support is only available for versions of atlas installed via XXX. This way, you wouldn't prevent the usage of atlas for expert users who prefer a different way of packaging and installing atlas. After all, compiling a Go binary is no rocket science...

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.

2 participants