API Version

[dillon-giacoppo]

Having multiple versions exist in the same repository is confusing and there are some minor typo bugs.

The atlassian fork leverages the GitHub release tag which can be locked for a specific Artifactory api version and will prevent fragmenting the api in the repo on each version.

It also contains support for more resources and fixes a few bugs

Instead of maintaining a seperate fork would you be keen to merge the fork back in?

Thanks

[jbrockopp]

@dillon-giacoppo @lusis I agree that instead of having all versions hosted in the same codebase at the same time, we should rather try to use GitHub tags to match them to specific versions of Artifactory.

However I also feel that we should consider doing a total refactor of the codebase. I feel we should strive towards using a services based model (like go-github) which is something more opensourced libraries seem to be moving towards today that I see.

Instead of one client that implements all of these methods, we instead try to segment the codebase with services that interact with specific endpoints of Artifactory. For example we might have a UserService that is for interacting with endpoints that are related to Artifactory users. Then we might have a RepositoryService that is for interacting with repository endpoints...

I'm willing to submit PRs and help out with this and I think this would be for the better of the community as well. Thoughts?

[jbrockopp]

Other Resources that I know of that use the services model (I'm sure there are plenty of more):

https://github.com/andygrunwald/cachet https://github.com/xanzy/go-gitlab https://github.com/andygrunwald/go-jira https://github.com/ktrysmt/go-bitbucket https://github.com/andygrunwald/go-gerrit

[lusis]

I'll take a look at the atlassian fork. I just did a huge refactor of my rundeck library that unifies the code base to a single package oath and supports versioning without the need for different branches or package paths. Let me look at the fork tonight and see if that's the way I'd like to go

[dillon-giacoppo]

Awesome! The atlassian fork has been stripped back to the bare minimum for our use case and was not intended as a comprehensive library (only as a client for a terraform provider) and so would not be a quick PR back in as a lot of the api was modified for simplicity. We would be happy to contribute back to this repo to add further endpoint support and fixes. The benefit of a GitHub release is the permanency of each version. Users can version lock on a GitHub tag instead of locking onto a branch, for instance, using dep:

[[constraint]] name = "github.com/atlassian/go-artifactory" version = "2.0.2"

[lusis]

So I'd love it if @dillon-giacoppo and @jbrockopp would take a gander at these two things:

https://github.com/lusis/go-rundeck/blob/master/README.md https://github.com/lusis/go-rundeck/tree/master/pkg/rundeck

This is the path I was planning on going down with the refactor of the go-artifactory client. I can still make tags at points in time but the general idea is not to NEED it. I would also do the break out of request/response parsing into its own subpackage like I did with rundeck. This is what made it much easier to allow for the single pkg/rundeck directory.

I was planning on starting that work this week.

[dillon-giacoppo]

@lusis is there a benefit to not putting the code at the root of the repository, it seems to be more conventional to have github.com/lusis/rundeck for the latest version. I am not sure how this removes the needs for tags as they are generally easier to deal with then locking a version than via sha. Also, regarding previous artifactory api versions, are you looking to maintain the older versions, or start from the latest and develop incrementally, which would be much easier to start with. Perhaps old versions should be on specific branches? It might be worthwhile creating a release branch for each major version (corresponding to artifactory api version perhaps) and then deprecating these branches on each breaking release. This means patches can be applied on previous versions and people can lock onto a compatible branch and will be guaranteed compatibility as well as all future patches (i.e security patch) with an artifactory api or a specific version on that branch, https://github.com/semantic-release/semantic-release is a good example of this pattern.

[dillon-giacoppo]

The model I am thinking looks like this:

version 4x of artifactory is on branch 'a' under library version 1.x.x and tagged as stale version 5x of artifactory is on branch 'b' under library version 2.x.x on release of version 6x, branch 'b' is tagged as stale and branch 'c' is the active branch (master) equivalent, releases are now under 3.x.x

Patches can still be applied to versions 4 of 5 by modifying their stale branch and creating a new release 1.1.x for example.

This would be very easy to implement from the latest version as it lends itself to being incremental. To create the right branch history and support 4x by splitting up the current branch would need a bit of git magic.

[lusis]

@dillon-giacoppo the main reason for the layout is one of personal consistency. At work we follow roughly this: https://peter.bourgon.org/go-best-practices-2016/#repository-structure (I know helm follows a similar pattern: https://github.com/kubernetes/helm)

As to the semver stuff, I'm not against that at all. I don't mind doing tags that people can pin to as they need. My point was more that this approach I've been taking to writing api clients has made it quite a bit easier to avoid breaking golang package api compatibility at the expense of api changes from the upstream thing.

Having maintained several api clients for various things, I've recently settled on the approach I'm using in the golang rundeck library:

• request/response parsing are in dedicated packages and support "versioning" through a min/max version per request/response type
• functions in the base package wrap request/response processing with function names as closely as possible aligning with the api docs naming (i.e. GetRepository)
• sugared functions/helpers are in a different package (i.e. a hypothetical CreateUserInGroup or CreateAdminUser).

By separating the concerns, people can pick and choose which parts of the library work best for them - they can just use the request/response types, the api mapped functions or the highlevel sugared stuff.

Let me get a sample out there for you to see in a branch that you can comment on.

[lusis]

Just one more thing to add, I've found through taking the response/request break-out approach that I've been able to separate the API's construct of an object from the higher level representation and that's resulted in more stable code (less risk of needing to break the function signatures to accomodate api changes for instance)

[lusis]

@dillon-giacoppo @jbrockopp take a look at:

https://github.com/lusis/go-artifactory/tree/single-dir

Specifically: https://github.com/lusis/go-artifactory/blob/single-dir/pkg/artifactory/user.go compared with the original implementation.

This will give you a feel for the direction I'm planning on going. Again, none of this means I can't do tagged releases at certain points but I've found that this approach let's me separate concerns more cleanly. Higher level constructs can be layered on top of the things in pkg/artifactory or even pkg/artifactory/requests and pkg/artifactory/responses.

The general idea is that sugared abstractions are entirely isolated and you get a maintainable versioning of the artifactory upstream API with the code base.

[dillon-giacoppo]

@lusis The layering of the library sounds great and I personally do not have particularly strong feelings to any repo layout, the document you linked me was a good read.

I do however feel that supporting multiple versions of api in the code would be a nightmare to maintain and inflates the codebase. What benefit does this provide over a good semver pattern?

From what I understand you are saying that if the api changes, a new conditional is added to support that change, and users bump there library version and everything should just work.

There are 3 kinds of changes though -

• Bug fixes which don't affect api
• Api endpoint gets added, which requires end-users to change their code if they want to use it
• Api endpoint gets modified or the model changes, which can either be:
  • URL changes, which is low level, won't affect end-users
  • The model changes which means end-users have to modify their code to work with the new model but this kind of change only occurs on a major version bump of artifactory which would mean there should be no expectation that end users code should be compatible.

I agree that api consistency is important and that the abstractions should rarely be modified but I don't see how supporting co-existing versions provides benefits to the end user as their code will need to change regardless if the api model changes. This feels like it would lead to a divergence from the artifactory api's model and the library api's model over time - I personally think they should be 1:1.

[dillon-giacoppo]

Perhaps a check should be performed that if the artifactory version is not consistent with the library's version, a warning is displayed, which I think is quite common practice. For me it just makes more sense that I pick the library version that is compatible with my artifactory, not expect any version of the library to support any version of artifactory api because of what will happen with model inconsistencies.

[lusis]

I just want to be clear that I'm still going to be creating tags or branches as checkpoints but the GOAL is that master degrades gracefully for users of older versions.

As to checking the version, I'm going to work on that as well. I'm thinking of just having the client set the api version at instantiation based on the api call to get the version. It can still be overridden but the default will be the result of an api call.

[lusis]

As for the details of the graceful degradation, I wrote a little bit about it here:

https://github.com/lusis/pkg/blob/master/versioner/README.md

[dillon-giacoppo]

@lusis appreciate the discussion :) I am not sure that there is a use case where a user would require master to degrade for them on an outdated version.

From the example provided in the pkg readme -

While they won't be able to call ListUsers, they can call GetSystemInfo.

This means that they will be pulling code that is not relevant for them, what they can use hasn't changed. If release branches are used, a patch can be retroactively applied if bug fixes are found. I find it unlikely that a new api method would come out that could be relevant for users of previous versions. The benefits of this is that the code base doesn't build up which leads to easier maintenance and is much clearer to the end user what they can and can't use. For instance IDE code support would not recognise that a method is not available for a particular version using the proposed versioning system.

It would appear that the only benefit this would have now is that it would be easier to initially refactor the library to support version 4 in the sense that adding new functions after the minimum viable product would need to be applied to one branch instead of two, but I see this more of a one time starting cost that prevents a lot of future tech debt. This is not an issue if the refactor starts with support for the current version upwards, there would be a really awesome opportunity to have a really clean version system. @jbrockopp - thoughts?

[jbrockopp]

I thought about this a lot for the past couple days and ultimately I have mixed feelings on the matter..

I have concerns with the fact that I'm wondering if this system is a little overkill for what we are trying to accomplish... Creating a simple, easy-to-use golang library for interfacing with the Artifactory API. I personally feel this will increase the complexity of the library which could deter possible consumers of it.

The layering of the library sounds great and I personally do not have particularly strong feelings to any repo layout, the document you linked me was a good read.

While I agree that I don't have strong feelings one way or the other, I think we need to be cautious of this. Consumers of the library will sacrifice complexity for simplicity. If someone comes into the code base, and is having a hard time deciphering the code or determining how something works, they may be inclined not to use it. We need to be careful that we aren't sacrificing personal consistency for usability.

This means that they will be pulling code that is not relevant for them, what they can use hasn't changed. If release branches are used, a patch can be retroactively applied if bug fixes are found. I find it unlikely that a new api method would come out that could be relevant for users of previous versions. The benefits of this is that the code base doesn't build up which leads to easier maintenance and is much clearer to the end user what they can and can't use. For instance IDE code support would not recognise that a method is not available for a particular version using the proposed versioning system.

I agree this is an issue for consumers of the library.

I do however feel that supporting multiple versions of api in the code would be a nightmare to maintain and inflates the codebase. What benefit does this provide over a good semver pattern?

I share this sentiment. I personally feel that semantic versioning is the way to go because I like the idea of being able to pin the library to a specific tag that correlates to a specific version of Artifactory rather than tracking down the sha in the codebase for the version of Artifactory I'm running. I've had issues in the past when interfacing with the Docker's API because of them not creating tagged versions specific to their API. This can create a burden on end users of the library and I think we need to be cautious about this.

I just want to be clear that I'm still going to be creating tags or branches as checkpoints but the GOAL is that master degrades gracefully for users of older versions.

However, it appears that we are going to still create tagged releases that correlate to the Artifactory versions. I think this if this is the case I'm willing to secede my above concerns in favor of seeing the actual implementation.

All in all, I'm anxious but excited to see where this library takes off. I'm going to hold off on any "nay-saying" until I see the actual implementation of the library and how it works. The practical usage of the library is going to be the determining factor for me.

Regards

[jbrockopp]

@lusis Any updates on this?

[jbrockopp]

@dillon-giacoppo @lusis

My team and I wanted to show you our version of a Golang client library for the Artifactory and Xray APIs.

We decided to go down this path for a few reasons:

• Project inactivity
• More pluggable design like https://github.com/google/go-github
• Addition of Xray resources
• fixes https://github.com/lusis/go-artifactory/issues/27

We just open sourced this project, and we’ve added attribution back to this library wherever we re-used the codebase.

We'd love for you to start consuming this project and contribute any features that you'd like to see.

The project can be found here: https://github.com/target/go-arty

Thanks!

[dillon-giacoppo]

Hi @jbrockopp,

Good to hear from you, some great stuff. Looks like it has an issue of not using pointers for struct fields meaning values can't be zeroed so I am not sure if it is a fix for #27. This is a small one that I wrote for a terraform provider which you may find useful https://github.com/atlassian/go-artifactory. (an example of the library being used is here https://github.com/atlassian/terraform-provider-artifactory). I haven't encountered any issues yet but I don't have comprehensive testing on it. It doesn't cover everything yet as I was adding components as I needed. There are some cheeky workarounds in there as the Artifactory api is not exactly as is in the documentation (coordinating with JFrog to get a V2 api release eventually).

Just FYI, we are coordinating a plan with JFrog to extract a library from the JFrog CLI tool which has a lot of usability commands like uploading files and combine it with the sys admin endpoints into a JFrog maintained go-client library. This will happen, if at all, after the V2 api as the current one has become a bit hairy. I wouldn't wait on this library any time soon, but something good to know :)

Thanks!!

[jbrockopp]

@dillon-giacoppo Sorry it took awhile for it to get implemented but your requested changes have been made here: https://github.com/target/go-arty/pull/12

If you have any thoughts/comments/questions feel free to reach out in that repository.

Thanks!