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

Update Readme.md: gcs to bq + cloud armor / glb #743

Merged
merged 24 commits into from
Aug 1, 2022
Merged

Update Readme.md: gcs to bq + cloud armor / glb #743

merged 24 commits into from
Aug 1, 2022

Conversation

bensadikgoogle
Copy link
Collaborator

No description provided.

@ludoo
Copy link
Collaborator

ludoo commented Jul 18, 2022

Can you make sure linting checks pass? The instructions are in our contributing guide in the top-level folder.

@apichick
Copy link
Collaborator

Both the old and the new diagram are missing the cloud nat instances in two regions

@bensadikgoogle
Copy link
Collaborator Author

@apichick thanks I'll look into it

@ludoo ludoo requested a review from lcaggio July 21, 2022 09:13
@bensadikgoogle
Copy link
Collaborator Author

@apichick in what architecture do you think we should add Cloud NAT ? I'm not seeing any mention of NAT in the written explanation. As I haven't worked on them but happy to discuss this if you have time

@ludoo ludoo requested a review from apichick July 21, 2022 09:39
@bensadikgoogle
Copy link
Collaborator Author

@ludoo @apichick do you have an idea on why this check fails?
Here are the content that generates failures (2 failed, 156 passed)
I'm not 100% sure I understand what this test step performs

FAILED tests/doc_examples/test_plan.py::test_example[net-glb:HTTPS And SSL Certificates 3]
FAILED tests/doc_examples/test_plan.py::test_example[net-ilb-l7:HTTPS And SSL Certificates 2]

@ludoo
Copy link
Collaborator

ludoo commented Jul 25, 2022

I just sent a PR that should fix it. :)

@bensadikgoogle
Copy link
Collaborator Author

@apichick thanks for the review I removed/changed the ambiguous sentences as you requested. Let me know if this works for you

@bensadikgoogle bensadikgoogle requested a review from apichick July 26, 2022 17:17
@bensadikgoogle bensadikgoogle requested a review from apichick July 27, 2022 14:13
@apichick
Copy link
Collaborator

GLB + armor changes look good to me @lcaggio dud you have the change to look at the gcs and bq changes?

@bensadikgoogle bensadikgoogle requested a review from ludoo August 1, 2022 13:39
@ludoo ludoo merged commit d2feb15 into GoogleCloudPlatform:master Aug 1, 2022
@apichick
Copy link
Collaborator

apichick commented Oct 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants