lyft
Refactoring code to refresh tokens
Fixing https://github.com/lyft/cartography/issues/1323 while refactoring token management
It will be a lot of work, but if you can use the new data model that will be great!
Example: https://github.com/lyft/cartography/blob/master/cartography/intel/aws/ec2/instances.py https://github.com/lyft/cartography/blob/master/cartography/models/aws/ec2/instances.py#L75
We can, but for a simple fix it would too much for review IMO. Even the changes in this PR seems a bit too much, but still might be worth it for readability. Would the changes in PR looks too far from convention?
@soumyadipDe Thanks for the PR. I need a bit more time to review closely since it changes the structure of the sync.
It'd be amazing to add unit tests to ensure that things are stitched together properly, but I realize the original code didn't have it to begin with. Basically it'd be good to have some assurance that this works and some demonstration of the new behavior -- if unit tests are too much work, can you include either a log or screenshot showing that this works on your machine?
It will be a lot of work, but if you can use the new data model that will be great!
Nah, that is out of scope of this PR since the goal here is to fix a credential refresh problem.
There's not much to show in logs as fix means there's no error. The end of it shows the time process had run for the whole process.
INFO:cartography.graph.job:Finished job gcp_dns_cleanup
INFO:cartography.graph.statement:Completed gcp_compute_asset_inet_exposure statement #1
INFO:cartography.graph.statement:Completed gcp_compute_asset_inet_exposure statement #2
INFO:cartography.graph.statement:Completed gcp_compute_asset_inet_exposure statement #3
INFO:cartography.graph.statement:Completed gcp_compute_asset_inet_exposure statement #4
INFO:cartography.graph.statement:Completed gcp_compute_asset_inet_exposure statement #5
INFO:cartography.graph.statement:Completed gcp_compute_asset_inet_exposure statement #6
INFO:cartography.graph.statement:Completed gcp_compute_asset_inet_exposure statement #7
INFO:cartography.graph.job:Finished job gcp_compute_asset_inet_exposure
INFO:cartography.graph.statement:Completed gcp_gke_asset_exposure statement #1
INFO:cartography.graph.statement:Completed gcp_gke_asset_exposure statement #2
INFO:cartography.graph.job:Finished job gcp_gke_asset_exposure
INFO:cartography.graph.statement:Completed gcp_gke_basic_auth statement #1
INFO:cartography.graph.statement:Completed gcp_gke_basic_auth statement #2
INFO:cartography.graph.job:Finished job gcp_gke_basic_auth
INFO:cartography.sync:Finishing sync stage 'gcp'
INFO:cartography.sync:Finishing sync with update tag '1720411083'
real 197m21.362s
user 6m20.901s
sys 0m15.675s
But this log was with this fix - https://github.com/lyft/cartography/pull/1324/files , not with this change with refactoring.
There was an update on the way authentication works to GCP at https://github.com/cartography-cncf/cartography/pull/1434. Hope it simplifies a bit this issue.
