Skip to content

Conversation

runakash
Copy link
Member

@runakash runakash commented Oct 21, 2021

Issue #, if available:

Description of changes: Handle DNS namespaces along-with the HttpNamespace. Create the Service for the DNS namespace type the type SRV. Update the namespaceCache to support namespace struct.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov-commenter
Copy link

codecov-commenter commented Oct 21, 2021

Codecov Report

Merging #48 (a06ecf7) into main (bfd82eb) will increase coverage by 4.38%.
The diff coverage is 83.96%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #48      +/-   ##
==========================================
+ Coverage   37.94%   42.32%   +4.38%     
==========================================
  Files           9        9              
  Lines         954      997      +43     
==========================================
+ Hits          362      422      +60     
+ Misses        579      560      -19     
- Partials       13       15       +2     
Impacted Files Coverage Δ
pkg/model/types.go 72.41% <0.00%> (-15.09%) ⬇️
pkg/cloudmap/client.go 53.71% <89.06%> (+4.17%) ⬆️
pkg/cloudmap/api.go 29.34% <100.00%> (+25.93%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bfd82eb...a06ecf7. Read the comment docs.

Copy link
Contributor

@vanekjar vanekjar left a comment

Choose a reason for hiding this comment

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

Thanks, it looks good! Can you please also rebase on top of the current main? @astaticvoid refactored bunch of interfaces and there may be minor adjustments needed.

@vanekjar
Copy link
Contributor

Linking #18

@vanekjar vanekjar linked an issue Oct 21, 2021 that may be closed by this pull request
@runakash runakash changed the title Handle DNS namespaces along-with the HttpNamespace. Create the Servic… Add the support for creating service with DNS namespace type Oct 22, 2021
Copy link
Contributor

@vanekjar vanekjar left a comment

Choose a reason for hiding this comment

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

Thanks for the update!

@runakash runakash force-pushed the dns-namespace branch 2 times, most recently from 87910f7 to a99707c Compare October 22, 2021 23:21
@runakash runakash requested a review from vanekjar October 22, 2021 23:55
@vanekjar
Copy link
Contributor

Looks good! I'll let @astaticvoid to check that as well before merging.

@vanekjar vanekjar requested review from astaticvoid and removed request for vanekjar October 23, 2021 04:46
…pace types into a enum. Separate constant for dns record ttl config. Add tests for the changes to api.go and caching logic.
…he logging for unsupported namespace types. Adding more tests.
@astaticvoid
Copy link
Contributor

Looks great, thanks for the discussions.

@runakash runakash merged commit ba9b31c into aws:main Oct 25, 2021
@runakash runakash deleted the dns-namespace branch October 25, 2021 21:48
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.

Handle both Cloud Map namespace types (DNS/HTTP)
4 participants