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

timing: a few simple tilt up cases #116

Merged
merged 4 commits into from Aug 21, 2018
Merged

Conversation

maiamcc
Copy link
Contributor

@maiamcc maiamcc commented Aug 20, 2018

Hello @jazzdan, @nicks,

the beginnings of a timing script that should hopefully be extensible n stuff

@maiamcc maiamcc requested review from jazzdan and nicks August 20, 2018 21:57
Copy link
Contributor

@jazzdan jazzdan left a comment

Choose a reason for hiding this comment

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

LGTM!

| _ < __/\__ \ |_| | | |_\__ \_
|_| \_\___||___/\__,_|_|\__|___(_)'''

BLORGLY_BACKEND_DIR = os.path.join(os.environ['GOPATH'], 'src/github.com/windmilleng/blorgly-backend')
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we be using blorg-backend for our benchmarks since @yuindustries discovered that blorgly-backend is kind of borked?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is it? how so? (i forgot about that.)

Copy link
Member

Choose a reason for hiding this comment

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

Blorgly-Backend is not actually hooked up to the frontend. The original idea was for it to provide random strings, but it neither generates those strings nor seems to have ever been successfully tested communicating with the frontend when everyone's deployed to K8s. (It was tested on local, though, and I assume it works there.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm so i feel like as long as it deploys (which it does), it doesn't matter if it works -- but also i don't have strong feelings and am happy to defer to others here

Copy link
Member

Choose a reason for hiding this comment

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

I think it's fine to use blorg-backend for now, since for milestone purposes I'd want anything related to output to be merged in sooner rather than later. And I agree with your logic.

I myself ended up using blorg-backend for the Docker for Mac test (PITA though it is, with its Cockroach DB setup and such) because I couldn't figure out another way of verifying that "everything works."

Copy link
Member

@nicks nicks left a comment

Choose a reason for hiding this comment

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

nice!

from subprocess import call

RESULTS_BLOCKLTR = ''' ____ _ _
| _ \ ___ ___ _ _| | |_ ___ _
Copy link
Member

Choose a reason for hiding this comment

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

🎩



def main():
print 'NOTE: this script doesn\'t install `tilt` for you, and relies on you having the ' \
Copy link
Member

Choose a reason for hiding this comment

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

wait are we on python3 yet

Copy link
Contributor Author

@maiamcc maiamcc Aug 20, 2018

Choose a reason for hiding this comment

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

uhhhh i guess we could be? i guess the blocker was fab, so that'd probs be fine. will make that change first thing tomorrow

| _ < __/\__ \ |_| | | |_\__ \_
|_| \_\___||___/\__,_|_|\__|___(_)'''

BLORGLY_BACKEND_DIR = os.path.join(os.environ['GOPATH'], 'src/github.com/windmilleng/blorgly-backend')
Copy link
Member

Choose a reason for hiding this comment

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

not for this pr, but i wonder if this will eventually become a more OO class where the current backend dir and service names are fields of the class

Copy link
Contributor Author

Choose a reason for hiding this comment

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

totes, that makes sense

@maiamcc maiamcc force-pushed the maiamcc/prelim-timing-script branch from a5309fb to d901da2 Compare August 21, 2018 16:07
- Our Python scripts are in Python 3.6.0. To run them:
- **[pyenv](https://github.com/pyenv/pyenv#installation)**
- **python**: `pyenv install`
- if you're using GKE and get the error: "pyenv: python2: command not found", run:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fyi @nicks @jazzdan this is a potentially awkward result of having our scripts be in py3. I've got stuff working fine now, but if we think this is too much of a PITA for ppl trying to run the scripts, we can easily change them back to py2. thoughts?

@maiamcc maiamcc merged commit 23de4e9 into master Aug 21, 2018
@maiamcc maiamcc deleted the maiamcc/prelim-timing-script branch August 21, 2018 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants