Skip to content

ADI export script (bug 1014752) #115

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.

Sign up for GitHub

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

Jump to bottom
Merged
yohanboniface merged 1 commit into from
Jun 12, 2014
Merged

ADI export script (bug 1014752) #115

yohanboniface merged 1 commit into from
Jun 12, 2014

Conversation

@magopian
Copy link
Contributor

@magopian magopian commented Jun 7, 2014

Fixes bug 1014752

@magopian magopian changed the title [WIP] ADI export script (bug 1014752) ADI export script (bug 1014752) Jun 11, 2014
yohanboniface
yohanboniface reviewed Jun 11, 2014
View reviewed changes
apps/addons/management/commands/download_counts_from_file.py Outdated
Copy link
Contributor

@yohanboniface yohanboniface Jun 11, 2014

Choose a reason for hiding this comment

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

Can't we default to the date we will be using (I guess "yesterday")?

Copy link
Contributor

@davidbgk davidbgk Jun 11, 2014

Choose a reason for hiding this comment

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

I prefer to be explicit on that one given the amount of time/resource it will consume.

Copy link
Contributor

@yohanboniface yohanboniface Jun 11, 2014

Choose a reason for hiding this comment

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

Fair enough :)

@yohanboniface
Copy link
Contributor

yohanboniface commented Jun 11, 2014

General note: we should consider doing a PR to pysh2 for the yieldcursor thing. It's worth it in IMHO as we are now adding it as a AMO dependency.

@davidbgk
Copy link
Contributor

davidbgk commented Jun 11, 2014

The PR for pyhs2 already exists: BradRuderman/pyhs2#17

yohanboniface
yohanboniface reviewed Jun 11, 2014
View reviewed changes
apps/addons/management/commands/download_metrics.py
Copy link
Contributor

@yohanboniface yohanboniface Jun 11, 2014

Choose a reason for hiding this comment

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

no need to obfuscate those credentials with a os.environ.get() or something like that?

Copy link
Contributor Author

@magopian magopian Jun 11, 2014

Choose a reason for hiding this comment

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

Those are only accessible through the vpn, so I believe this is ok, please let us know if it isn't the case @jasonthomas

yohanboniface
yohanboniface reviewed Jun 12, 2014
View reviewed changes
apps/addons/management/commands/update_counts_from_file.py
Copy link
Contributor

@yohanboniface yohanboniface Jun 12, 2014

Choose a reason for hiding this comment

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

Maybe we wanna log this king of skip? In case we have some counter weirdness once in prod, this may help.

yohanboniface added a commit that referenced this pull request Jun 12, 2014
@yohanboniface
Merge pull request #115 from magopian/1014752-adi-export-script
dcfbeb2 
ADI export script (bug 1014752)
@yohanboniface yohanboniface merged commit dcfbeb2 into mozilla:master Jun 12, 2014
@DavidBruant DavidBruant mentioned this pull request Jun 16, 2016
Open
@smathot smathot mentioned this pull request Aug 5, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@magopian @yohanboniface @davidbgk @jasonthomas