Asyncio Listing and Inventory Report Integration

[hanseaston]

@martindurant

I've completed the integration of the inventory report service and asyncio listing logic based on our discussions.

The overview of the logic is as follows:

For listing, the user can optionally pass in a inventory_report_info argument as part of the kwargs.

If the argument is not passed in, then the listing logic is exactly the same as previously, thus it is backward compatible with existing implementation.

If the argument is passed in, the high-level is as follows:

1. Validate whether inventory_report_info has the correct fields included to fetch the inventory report.
2. Fetch the inventory report configuration.
3. Parse the inventory report configuration to validate and extract the inventory report location.
4. Does listing on the inventory report metadata.
5. Use the metadata to fetch the latest inventory report(s).
6. Parse the inventory report content and extract objects metadata.
7. If the user wants to directly use the inventory report for listing, then return the objects metadata.
8. If the user wants to retrieve the most up-to-date listing, then use the object names from the inventory report to divide up workload for each coroutine fast listing.

The listing performance is ~10x from my own experiments.

Latency for 1 - 6 should be trivial since:

1. Fetching the inventory report is very fast, a single call.
2. Performing listing on inventory is very fast, most likely a single call (5000 inventory reports metadata on a single page).
3. Downloading inventory report is relatively past (inventory reports with 1M objects is ~40MB).

In addition, if the user chooses to use the inventory report content directly for listing results, the speedup can be >100x.

Comments have been added throughout the implementation, and there are comprehensive test cases covering each method in the InventoryReport class.

Would really appreciate it if any of you has any feedback, so that we can have this awesome feature integrated!

[bernardhan33]

Hi all, this is Bernard Han from Google GCS Team. I'm Hans' co-host this summer. Leaving some general comments on this public PR.

[bernardhan33]

Also, are README changes in a separate commit or PR?

[martindurant]

Everything passes, so that's a good start :)

I will start looking at this later today - there is quite a lot! Sorry for being slow getting here, got hit by covid.

[hanseaston]

Everything passes, so that's a good start :)
I will start looking at this later today - there is quite a lot! Sorry for being slow getting here, got hit by covid.

Oh no sorry to hear, covid sucks ;(

[hanseaston]

Also, are README changes in a separate commit or PR?

I am thinking of adding a README change, and maybe a blog post on how to use the inventory report, once the PR is officially merged (in order to finalize all the details).

[martindurant]

Yes, we certainly need prose "howto" documentation around this, in our own docs source. I would recommend any blog focus on the process of development and speedup for your example case rather that a how-to-use piece. Of course, the two can link to one-another.

I don't mind if the docs lag behind this PR, effectively making the new feature experimental. In fact, calling it an experimental feature isn't a bad idea, since we don't have a good feel of the set of use cases it might be pulled into, and there may be some that behave badly.

[hanseaston]

Yes, we certainly need prose "howto" documentation around this, in our own docs source. I would recommend any blog focus on the process of development and speedup for your example case rather that a how-to-use piece. Of course, the two can link to one-another.

Sounds good, I will write up a prose (internally) and a blog (shared externally) once all the changes are finalized for this PR. Meanwhile, should I add a couple comments on top of the inventory report logic, indicating this is currently experimental?

[hanseaston]

Update: I fixed the typos that Bernard has pointed out in the latest commit @Magichan33.

[martindurant]

should I add a couple comments on top of the inventory report logic, indicating this is currently experimental?

Yes, I think so

[hanseaston]

should I add a couple comments on top of the inventory report logic, indicating this is currently experimental?

Yes, I think so

Sounds good, just updated that.

[martindurant]

Just small things left

[martindurant]

👍