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

Add Process Cloud Address #30

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

MichelDiz
Copy link
Contributor

I added the ProcessAddress method in the Dgraph Cloud Channel to properly handle various address formats. The method now ensures that the address is correctly formatted for Dgraph Cloud by extracting the required components from the input and constructing the expected gRPC address. I also added error handling to ensure that only valid cloud.dgraph.io addresses are processed.

Also, I have created a test suite with various test cases to validate the functionality of the updated method and ensure it works as expected.

@MichelDiz MichelDiz added the enhancement New feature or request label Apr 18, 2023
address = new Uri(address).Host;
}

var match = Regex.Match(address, @"^(?<name>[\w-]+)\.(?<region>[\w-]+)\.aws\.cloud\.dgraph\.io");

Choose a reason for hiding this comment

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

I am not sure regex is the best way. Regex usually are complex and hard to understand like, I have no idea what this regex is doing. Also, it feels like aws has to be in the URL which may not be necessary.

Copy link
Contributor Author

@MichelDiz MichelDiz Jul 13, 2023

Choose a reason for hiding this comment

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

This regex is used to match and parse a specific structure of string, which it's used for matching the address of Dgraph instances on AWS cloud.

PS. With this regex alone I'm saving probably 30 lines of code or more.
And we use Regex in important places in the main code and others. There's no issue with that if you understand what the regex does.

This is a regular address in the cloud https://green-fog.us-east-1.aws.cloud.dgraph.io/graphql
We don't have any other pattern in the cloud. So the idea is to make sure the address is 100% correct.

As you can see, I have added a test. In that test I'm validating a variation of URLs.

Here is a breakdown of the regular expression:

  • ^: This asserts the start of a line.
  • (?<name>[\w-]+): This is a named capture group named 'name'. The [\w-]+ means it matches any word character (equal to [a-zA-Z0-9_]) or hyphen, one or more times. So it can capture the name of the instance like 'my-instance'.
  • \.: This matches the literal '.' character.
  • (?<region>[\w-]+): This is another named capture group named 'region'. It works the same as the 'name' group, capturing the region of the instance like 'us-west-1'.
  • \.aws\.cloud\.dgraph\.io: This matches the literal string '.aws.cloud.dgraph.io'.

So, the regex matches strings that start with a word (which can contain letters, numbers, underscores, or hyphens), followed by a '.', then another word (with the same possible set of characters), followed by the literal string '.aws.cloud.dgraph.io'.

For example, the string "my-instance.us-west-1.aws.cloud.dgraph.io" would be a match for this regex. The 'name' group would capture 'my-instance' and the 'region' group would capture 'us-west-1'.

@mangalaman93 mangalaman93 self-requested a review July 13, 2023 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants