Run find package & register and download & register as single command#1919
Run find package & register and download & register as single command#1919david-staniec-octopus wants to merge 4 commits into
Conversation
david-staniec-octopus
force-pushed
the
ds/registrationChanges
branch
9 times, most recently
from
edea829 to
7d77313
Compare
david-staniec-octopus
changed the title
david-staniec-octopus
changed the title
Jtango18
left a comment
Jtango18
left a comment
There was a problem hiding this comment.
Nothing particular scary or problematic in here to my 👀
david-staniec-octopus
force-pushed
the
ds/registrationChanges
branch
from
7d77313 to
d8a5311
Compare
| readonly static string tentacleHome = TestEnvironment.GetTestPath("temp", "FindAndRegisterPackage"); | ||
| readonly static string downloadPath = Path.Combine(tentacleHome, "Files"); |
There was a problem hiding this comment.
I have a visceral reaction to statics in tests
| [OneTimeSetUp] | ||
| public void TestFixtureSetUp() | ||
| { | ||
| Environment.SetEnvironmentVariable("TentacleHome", tentacleHome); |
There was a problem hiding this comment.
OMG, 😢
well I guess this is how it is.
| result.AssertFoundPackageServiceMessage(); | ||
|
|
||
| // Verify package was registered with the journal | ||
| result.AssertOutput("Registered package use/lock for {0} v{1} and task ServerTasks-12345", packageId, packageVersion); |
There was a problem hiding this comment.
Is it possible to instead assert that the package is indeed locked?
There was a problem hiding this comment.
See latest commit, I had some issues with reading from the repository (we don't seem to have similar tests for the existing register package command). I've fallen back on reading the JSON from the package cache I can see being written to the file system - thoughts?
| [SetUp] | ||
| public void SetUp() | ||
| { | ||
| if (!Directory.Exists(TentacleHome)) |
There was a problem hiding this comment.
Should we clear this in the setup so it is cleared on every test run?
There was a problem hiding this comment.
Sure, not a bad idea. It's in the latest commit.
|
It looks like the race condition remains between download/find and the following registration. What this PR does is significantly reduce the time between those two operations reducing the changes of someone cleaning up the package we just got. If we had more time we would also ensure that the cleanup operation never deletes files downloaded within the last few seconds. |
3 participants
The find package and download package commands do not register the package; this is done via a separate register command.
It is possible for other deployments to remove the packages via package retention prior to them being registered. Therefore, this PR introduces Find and Register Package and Download and Register package commands. There has been some refactoring to make the code common between the original commands and the new command, in order to address risk of divergence.
Server changes are required to use the new commands, but the existing commands will continue working without corresponding Server changes.