Start of the new user layer: client, daemon, registries #497
Conversation
…and authentication with registries.
|
This looks good, would it be possible to have just one .editorconfig at solution level? |
|
If we can reach a consensus on what the style rules should be, that would be best, yeah. 😄 |
|
Build failure is because I'm setting LoadUserProfile, which is Windows-only. |
| <EnableNETAnalyzers>true</EnableNETAnalyzers> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> |
There was a problem hiding this comment.
.editorConfig file is commited but removed from project here, is that what you want to do?
There was a problem hiding this comment.
I just didn't want to see it in Solution Explorer. It doesn't seem to effect whether it's respected.
test/DockerSdk.Tests/Cli.cs
Outdated
| /// </exception> | ||
| public static string[] Run(string command, bool ignoreErrors) | ||
| { | ||
| var pi = new ProcessStartInfo("pwsh.exe", "-Command -") |
There was a problem hiding this comment.
Haven't tested this yet, but would it be compatible with any OS?
Will check later what you are trying to do here
There was a problem hiding this comment.
Using PowerShell? Yeah, the docs say that it's installed on the runner.
That said, the current build failure is probably because of the .exe extension. I'll need to adjust that.
There was a problem hiding this comment.
OK, fixed. The tests that use this code all pass now.
| @@ -0,0 +1,73 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
There was a problem hiding this comment.
Configuration for the CodeMaid extension. IIRC, these are all the default values.
There was a problem hiding this comment.
I'm not sure if this should be part of the repository, I use codemaid extension and have a local .editorconfig myself but wouldn't commit it, same as I wouldn't commit my local color theme preferences
I may be wrong on this, this is my first time as contributor and is is taking time to learn some contributor etiquette
There was a problem hiding this comment.
I get where you're coming from, and certainly settings that only affect developers at an individual level shouldn't be checked in. .editorconfig can certainly be used to help make one's code consistent, but I think it's primarily meant to declare a team's desired conventions. It's no fun to open a project and find a thousand warnings because another author has broader variance. Committing an .editorconfig sets the baseline so everyone can build the code without warnings.
CodeMaid is much farther into the gray area. On further reflection, I agree that it shouldn't be checked in for OSS projects. I'll remove it.
| /// The command either wrote to stdout or returned a non-zero exit code, and <paramref name="ignoreErrors"/> is | ||
| /// false. | ||
| /// </exception> | ||
| public static string[] Run(string command, bool ignoreErrors) |
There was a problem hiding this comment.
This would add as requirement having powershell installed for development.
What on exceptions other than System.ComponentModel.Win32Exception?
There was a problem hiding this comment.
Yeah. I went with PowerShell because it's available on all major platforms with no fuss, and is already installed on the runner.
There was a problem hiding this comment.
The only other exception that wouldn't be a bug in this code is platform-not-supported. But I don't expect anyone to run the tests on a Nano server :)
| @@ -0,0 +1,18 @@ | |||
| # Sets up the docker containers needed by the project's tests. | |||
There was a problem hiding this comment.
Have you tried to do this by using different containers? So you don't require to run compose, and you could even run tests on a remote machine
There was a problem hiding this comment.
I hadn't considered a remote machine. That's an interesting idea!
It would definitely be nicer to containerize the testing, but I ran into some troubles and decided to give it up for now. I think it's a little beyond my current Dockerization skill level. I'd be pleased if someone more familiar with containerized tests would revamp this at some point, though. In particular, if we could get some Docker-in-Docker going, we could test against different daemon versions/configuration.
The main problem I had is that I don't want to give up the ability to debug tests from Visual Studio. VS has the ability to debug a program in a container, but as far as I could tell it doesn't run tests in a container.
There was a problem hiding this comment.
@Emdot I could help you on that, if you ever want to run tests on a dockerized environment, but first I need your help.
I hit a wall on #487
Can't cancel a stream.Read() call once it is reading the stream, because inside the stream reader it is using stream.read() and there is no cancellation token there.
And that's why they are using stream.Dispose() to cancel a stream read operation. This is what i suspect but is not confirmed.
The truth is tests aren't working because cancellation token is ignored
|
I deleted my fork to recreate it, and now it seems that I can no longer make edits to the branch. So I'm deleting this PR and will create an identical one in its place. |
|
The new PR is at #501 |
Start of the new user layer. Adds the client, connection to daemons, and authentication with registries. I'm calling it an "SDK" because that's what Docker calls the libraries for Go and Python.
Consumers would use code like the following to get started:
The only change to existing code is to make DockerClientConfiguration.LocalDockerUri public.
This includes test cases. For the registries portion it stands up some containers to test against--these containers run Docker registries. Presently there's not much testing around identity tokens.