Skip to content

Conversation

@IvanKolchanov
Copy link
Contributor

@IvanKolchanov IvanKolchanov commented Jul 17, 2024

Added endpoint and tests for it, had to change Node in Catalog.cs because it lacked a field
Resolves #349

IvanKolchanov and others added 30 commits June 11, 2024 05:11
….cs, added possible test, changed test_config for the test
…alNumber of Root class, due to a fail at version 1.6.10
…an 1.7.0

The field seems to be initialized to 0 in versions <= 1.7.0, which was causing a testing error
…s, modified IAgentEndpoint.cs to have GetCALeaf
…overload to ConnectAuthorize to support WriteOptions
Had to Add TerminatingGatewayConfigEntry and IngressGatewayConfigEntry classes for testing purposes
…ed Configuration.cs to have correct classes

Changed name TerminalGatewayEntry to TerminatingGatewayEntry
Copy link
Contributor

@marcin-krystianc marcin-krystianc 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. Just a few minor comments.

{
Name = "foo-ingress-gateway",
Listeners = new List<GatewayListener>
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Odd formatting, should be:

                Listeners = new List<GatewayListener>
                {
                ...


var services = await _client.Health.Ingress("foo-ingress", "", false);
Assert.Single(services.Response);
Assert.Equal("dc1", services.Response[0].Node.Datacenter);
Copy link
Contributor

Choose a reason for hiding this comment

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

It is better to assert that the data center is not empty ( to make the test independent from external configuration).

@IvanKolchanov
Copy link
Contributor Author

@marcin-krystianc fixed everything, I think

Copy link
Contributor

@marcin-krystianc marcin-krystianc left a comment

Choose a reason for hiding this comment

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

Nice, thank you.

@marcin-krystianc marcin-krystianc marked this pull request as ready for review July 18, 2024 14:02
@marcin-krystianc marcin-krystianc merged commit 91d20f8 into G-Research:master Jul 18, 2024
@IvanKolchanov IvanKolchanov changed the title Add List Ingress Health for Service Add GET List Ingress Health for Service Aug 2, 2024
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.

[FEATURE REQUEST]: List Ingress Health for Service

3 participants