Skip to content
This repository was archived by the owner on Oct 21, 2022. It is now read-only.

Record release time data into bigquery#46

Merged
liyanhui1228 merged 4 commits into
GoogleCloudPlatform:masterfrom
liyanhui1228:get_release_data
Jul 16, 2018
Merged

Record release time data into bigquery#46
liyanhui1228 merged 4 commits into
GoogleCloudPlatform:masterfrom
liyanhui1228:get_release_data

Conversation

@liyanhui1228
Copy link
Copy Markdown
Member

@liyanhui1228 liyanhui1228 commented Jul 13, 2018

Closes #45

"""Converts a CompatibilityResult into a dict which is a row for
release time table."""
if len(cs.packages) != 1 or cs.dependency_info is None:
return []
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this be an exception?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

As we only need to write the dependency_info in self compatibility result to release time table, so this is for skipping the pairwise compatibility result and the case when dependency_info is None, which is not an exception.

'dep_name': pkg,
}
row.update(version_info)
row['timestamp'] = row.pop('current_time')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you need to pop this? Can't you just use the value?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is for changing the key name from current_time to timestamp in the dict, as current_time is a reserved variable in bigquery.


@staticmethod
def _compatibility_status_to_release_time_row(
cs: CompatibilityResult) -> List:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You should specify the type of the list

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done.

cs: CompatibilityResult) -> List:
"""Converts a CompatibilityResult into a dict which is a row for
release time table."""
if len(cs.packages) != 1 or cs.dependency_info is None:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you add some documentation for dependency_info around line 49 - I can't remember that it is supposed to be.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done.

# record the py3 version by default. And also record py2 only packages.
for cs in compatibility_statuses:
if len(cs.packages) == 1:
if cs.python_major_version == '3' or \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think that I would do this in a slightly different way - create a dict mapping install_name to row and build the dict using this loop. Then do a insert_row on the values() of the dict.

That way you don't have to have any version-specific logic.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion! Done.

@liyanhui1228 liyanhui1228 merged commit ad58a20 into GoogleCloudPlatform:master Jul 16, 2018
@liyanhui1228 liyanhui1228 deleted the get_release_data branch July 16, 2018 21:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants