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

if not specified in HOCON, set AppVersion to executing assembly version #4618

Merged
merged 6 commits into from Dec 16, 2020

Conversation

Aaronontheweb
Copy link
Member

just so users don't have to manage application versions in two places -have Akka.Cluster.ClusterSettings automatically use the entry assembly's 'Major.Minor.Revision' version - that way if the developers are automatically bumping their entry executable's version, that is what will be reflected in HOCON.

If the user takes the time to manually input their akka.cluster.app-version - that value will be used instead.

cc @zbynek001

just so users don't have to manage application versions in two places -have Akka.Cluster.ClusterSettings automatically use the entry assembly's 'Major.Minor.Revision' version - that way if the developers are automatically bumping their entry executable's version, that is what will be reflected in HOCON.

If the user takes the time to manually input their akka.cluster.app-version - that value will be used instead.
@Aaronontheweb
Copy link
Member Author

Looks like we have a compilation failure here after 35 minutes? That's weird...

@Aaronontheweb
Copy link
Member Author

@Aaronontheweb
Copy link
Member Author

looks like the hung build was a fluke - ran fine the second time

@Aaronontheweb
Copy link
Member Author

Looks like there's an error with how the revision number gets used in this PR - but so far this feature is working very nicely.

@zbynek001
Copy link
Contributor

Not sure, but it might be better to hide this functionality behind some config switch.
In some scenarios, e.g. when using different assemblies joining the same cluster, it might have some unexpected behavior

@Aaronontheweb
Copy link
Member Author

ter to hide this functionality behind some config switch.
In some scenarios, e.g. when using different assemblies joining the same cluster, it mig

good point - I'll change the configuration around and to the following:

  1. Have a app-version = assembly config by default, which executes the app-version-from-assembly behavior;
  2. If that value is overridden with anything else, the explicit config value gets used instead.

Does that seem like a reasonable compromise?

@Aaronontheweb Aaronontheweb added this to In Review in Dec 14-25 2020 Sprint Dec 14, 2020
AppVersion now derives its value from `Assembly.GetEntryAssembly()` or `Assembly.GetExecutingAssembly()` (in case the former is `null`) by default.

The `akka.cluster.app-version` now defaults to `assembly-version` by default, which sets this behavior. To change that, simply use a different value.
@Aaronontheweb Aaronontheweb added this to the 1.4.13 milestone Dec 15, 2020
Copy link
Member Author

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

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

Left some comments describing the most recent round of changes

# app-version = "1.1-beta1"
# app-version = "1"
# app-version = "1.1"
app-version = assembly-version
Copy link
Member Author

Choose a reason for hiding this comment

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

Changed the default value here to assembly-version, which activates this assembly-scanning default behavior

}

public static AppVersion Create(string version)
{
var v = new AppVersion(version);
// check to see if we're going to use the assembly-version
if (version.Equals(AssemblyVersionMarker, StringComparison.InvariantCultureIgnoreCase))
Copy link
Member Author

Choose a reason for hiding this comment

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

Check to see if we're using assembly-version - if so, get the assembly data. If not, parse the version string as usual.


// if the entryAssembly is null (which can happen when we're called from unmanaged code)
// then fall back to the executing assembly for the version number.
var targetAssembly = entryAssembly ?? Assembly.GetExecutingAssembly();
Copy link
Member Author

Choose a reason for hiding this comment

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

Grab the EntryAssembly preferably. If this value is null, which can happen when called from unmanaged code (this happens when the XUnit test runner is executing from within visual studio, for instance) then fallback to the ExecutingAssembly version. Shouldn't happen in most Akka.NET applications, which tend to be launched as 100% managed processes.

@Arkatufus Arkatufus self-requested a review December 16, 2020 16:38
Copy link
Contributor

@Arkatufus Arkatufus left a comment

Choose a reason for hiding this comment

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

Looks good

@Arkatufus Arkatufus merged commit 1c50e4a into akkadotnet:dev Dec 16, 2020
Dec 14-25 2020 Sprint automation moved this from In Review to Done Dec 16, 2020
@Aaronontheweb Aaronontheweb deleted the feature/Automatic-AppVersion branch December 16, 2020 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants