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

Refactoring/gcp/iam #230

Merged
merged 45 commits into from
Apr 4, 2019
Merged

Refactoring/gcp/iam #230

merged 45 commits into from
Apr 4, 2019

Conversation

Remi05
Copy link
Contributor

@Remi05 Remi05 commented Mar 7, 2019

Migrated GCP's IAM service to the new architecture.

Note that gcp/resources/projects.py and gcp/facade/base.py are the same as in the Stackdriver logging PR; they are simply shared by all services and will appear in all GCP refactoring PRs until one of the services is merged.

@Remi05 Remi05 added this to the Iteration #4 milestone Mar 7, 2019
@Remi05 Remi05 self-assigned this Mar 7, 2019
@Remi05 Remi05 requested review from x4v13r64, Aboisier and misg March 7, 2019 04:08
@Remi05
Copy link
Contributor Author

Remi05 commented Mar 7, 2019

The service seems to work fine but the resource counts have changed; In the previous implementation, IAM showed a count of 8 resources where as the new implementation shows 4 resources. In both cases, there are 4 service accounts, which is shown correctly in the top navigation bar for both implementations. I thought it might be due to the keys and bindings counts but they do not seem to be set, even in the previous implementation.

Before:
iam_beforerefactoring

After:
iam_afterrefactoring

@Remi05
Copy link
Contributor Author

Remi05 commented Mar 7, 2019

The build doesn't pass for Python 3.5 because of formatted strings using the f'Text {my_variable}' syntax which was introduced in Python 3.6. Do we still plan on supporting Python 3.5 or only 3.6 and 3.7?

@x4v13r64
Copy link
Collaborator

x4v13r64 commented Mar 7, 2019

Do we still plan on supporting Python 3.5 or only 3.6 and 3.7?

Well asyncio is available from 3.5+, but 3.6+ should be the standard 3.x on most distros. If there's a benefit to only supporting 3.6+ that's something we could consider. I'm seeing a lot of changes happening in python 3.x so if we're along for the ride (and everyone will have to be from 2020 onward) then we might as well just accept that having outdated versions of python is not acceptable anymore.

@Aboisier
Copy link
Contributor

Aboisier commented Mar 7, 2019

Do we still plan on supporting Python 3.5 or only 3.6 and 3.7?

Well asyncio is available from 3.5+, but 3.6+ should be the standard 3.x on most distros. If there's a benefit to only supporting 3.6+ that's something we could consider. I'm seeing a lot of changes happening in python 3.x so if we're along for the ride (and everyone will have to be from 2020 onward) then we might as well just accept that having outdated versions of python is not acceptable anymore.

I'm not sure formatted string literals is a good enough reason to deprecate 3.5... but at the same time, it seems like there's barely anyone using 3.5.

image

@Remi05 Remi05 requested a review from misg March 14, 2019 18:46
Copy link
Contributor

@Aboisier Aboisier left a comment

Choose a reason for hiding this comment

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

Gg! Does it break the UI as well though?

@Remi05 Remi05 changed the base branch from refactoring/resource-configs to develop April 2, 2019 01:16
@Remi05
Copy link
Contributor Author

Remi05 commented Apr 2, 2019

Fixed UI and added left menu for projects.

Before :

iam_before

After :

iam_after

@Remi05 Remi05 requested a review from Aboisier April 2, 2019 02:38
@Remi05 Remi05 changed the base branch from develop to refactoring/gcp/stackdriverlogging April 2, 2019 02:52
Copy link
Contributor

@misg misg left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Contributor

@Aboisier Aboisier left a comment

Choose a reason for hiding this comment

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

I'd just add a small comment in the parse thingy, otherwise, gg!

@Remi05 Remi05 changed the base branch from refactoring/gcp/stackdriverlogging to develop April 4, 2019 01:51
@Remi05 Remi05 merged commit 902fda0 into develop Apr 4, 2019
@Remi05 Remi05 deleted the refactoring/gcp/iam branch April 4, 2019 01:53
@Aboisier Aboisier mentioned this pull request Apr 4, 2019
53 tasks
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.

5 participants