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

Adding token generation before each service API #1324

Merged
merged 4 commits into from
Oct 11, 2024

Conversation

soumyadipDe
Copy link
Contributor

Fix for #1323

@chandanchowdhury chandanchowdhury added the GCP GCP related issues and PRs label Jun 30, 2024
@chandanchowdhury
Copy link
Collaborator

I wonder if the solution would simpler if we

  • Move all _get_<Type>_resource to their individual files. Example: _get_compute_resource into cartography/intel/gcp/compute.py
  • Pass credentials along with project_id to all type specific code so it can do everything necessary

Basically, __init__.py, finds all projects and then for each project triggers type specific code which does the necessary work and handle all situations.

This way

  • we avoid needing to add _sync_single_project_<Type> in __init__.py
  • Make testing each type simple as the individual code takes care of everything related to a project and type

Hope I have not confused things.

@soumyadipDe
Copy link
Contributor Author

I tried the suggestion but as feared it ended up with many changes to the code. Creating another PR to check - #1325

This is still raw and I need to test the change, but wanted to get early review if this is a desired approach to this.

@chandanchowdhury
Copy link
Collaborator

@soumyadipDe just checking if you had a chance to test the changes? It looks good to me.

@soumyadipDe
Copy link
Contributor Author

Yes we have this code running in our GCP env and working as expected.

@chandanchowdhury chandanchowdhury merged commit 05916cd into cartography-cncf:master Oct 11, 2024
5 checks passed
danbrauer pushed a commit to etsy/cartography that referenced this pull request Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GCP GCP related issues and PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants