Skip to content
This repository has been archived by the owner on Dec 5, 2019. It is now read-only.

Bug 1308712 - Add Spark job detail view #35

Merged
merged 1 commit into from
Oct 11, 2016

Conversation

maurodoglio
Copy link
Contributor

@maurodoglio maurodoglio commented Oct 11, 2016

This also fixes a few bugs in the spark job startup code.


This change is Reviewable

Copy link
Contributor

@jezdez jezdez left a comment

Choose a reason for hiding this comment

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

rwc+


from session_csrf import anonymous_csrf

from atmo.jobs.models import SparkJob
Copy link
Contributor

Choose a reason for hiding this comment

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

mind doing from .models import SparkJob here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@@ -60,7 +60,7 @@
{% for spark_job in user_spark_jobs %}
<tr>
<td class="hidden">{{ spark_job.id }}</td>
<td>{{ spark_job.identifier }}</td>
<td><a href="{% url "jobs-detail" id=spark_job.id %}">{{ spark_job.identifier }}</a></td>
Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be cool to define a get_absolute_url in the SparkJob and use that instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

<p class="lead">Summary:</p>
<div class="row">
<div class="col-md-2"><strong>Notebook S3 key:</strong></div>
<div class="col-md-10">{{ job.notebook_s3_key}}</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

whitespace before the }} please, multiple instances of that in this file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@@ -72,7 +72,7 @@ def spark_job_run(user_email, identifier, notebook_uri, result_is_public, size,
'Path': 's3://{}/bootstrap/telemetry.sh'.format(
settings.AWS_CONFIG['SPARK_EMR_BUCKET']
),
'Args': ['--timeout', job_timeout]
'Args': ['--timeout', str(job_timeout)]
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

</div>
<div class="row">
<div class="col-md-2"><strong>Start date:</strong></div>
<div class="col-md-10">{{ job.start_date}}</div>
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 format the start and end dates somehow nicely?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

</div>
<div class="row">
<div class="col-md-2"><strong>Is enabled:</strong></div>
<div class="col-md-10">{{ job.is_enabled}}</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe how a checkmark glyph here if is_enabled is True?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@@ -0,0 +1,43 @@
{% extends "atmo/base.html" %}
{% block content %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Mind providing a head_title block as well? it'll be used in the <title> element

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@@ -134,10 +134,9 @@ def delete(self, *args, **kwargs):
@classmethod
def step_all(cls):
"""Run all the scheduled tasks that are supposed to run."""
now = datetime.now()
Copy link
Contributor

Choose a reason for hiding this comment

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

the import needs to be removed, too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@maurodoglio maurodoglio force-pushed the bug-1308712-job-detail branch from 47df61e to aada9df Compare October 11, 2016 10:31
@codecov-io
Copy link

codecov-io commented Oct 11, 2016

Current coverage is 78.81% (diff: 58.33%)

Merging #35 into master will increase coverage by 0.02%

@@             master        #35   diff @@
==========================================
  Files            41         41          
  Lines           971        977     +6   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            765        770     +5   
- Misses          206        207     +1   
  Partials          0          0          

Powered by Codecov. Last update 798a8bf...6cef0a8

@maurodoglio maurodoglio force-pushed the bug-1308712-job-detail branch from aada9df to 86314a4 Compare October 11, 2016 10:32
This also fixes a few bugs in the spark job startup code and in the
cluster-detail template.
@maurodoglio maurodoglio force-pushed the bug-1308712-job-detail branch from 86314a4 to 6cef0a8 Compare October 11, 2016 10:49
@maurodoglio maurodoglio merged commit 6333729 into master Oct 11, 2016
@jezdez jezdez deleted the bug-1308712-job-detail branch November 17, 2016 13:56
jezdez pushed a commit that referenced this pull request Jan 16, 2019
Bumps [cryptiles](https://github.com/hapijs/cryptiles) from 3.1.2 to 3.1.4. **This update includes security fixes.**
<details>
<summary>Vulnerabilities fixed</summary>

*Sourced from [The npm Advisory Database](https://npmjs.com/advisories/720).*

> **Insufficient Entropy**
> Versions of `cryptiles` from version 3.1.0 through 3.1.2, and versions 4.0.0 to version 4.1.1 are vulnerable to insufficient entropy.  The `randomDigits` method generates digits that lack a perfect distribution over enough attempts.
> 
> Affected versions: >=3.1.0 <3.1.3; >=4.0.0 <4.1.2

</details>
<details>
<summary>Commits</summary>

- [`52f2f68`](hapijs/cryptiles@52f2f68) 3.1.4
- [`88d8409`](hapijs/cryptiles@88d8409) Remove engines. Closes [#37](https://github-redirect.dependabot.com/hapijs/cryptiles/issues/37)
- [`2c3dcdf`](hapijs/cryptiles@2c3dcdf) node 10 globals
- [`4340e43`](hapijs/cryptiles@4340e43) Drop node latest
- [`7845877`](hapijs/cryptiles@7845877) Increase timeout
- [`2d626ed`](hapijs/cryptiles@2d626ed) 3.1.3
- [`cb6bd64`](hapijs/cryptiles@cb6bd64) Backport fix [#34](https://github-redirect.dependabot.com/hapijs/cryptiles/issues/34). Closes [#35](https://github-redirect.dependabot.com/hapijs/cryptiles/issues/35)
- See full diff in [compare view](hapijs/cryptiles@v3.1.2...v3.1.4)
</details>
<br />

[![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=cryptiles&package-manager=npm_and_yarn&previous-version=3.1.2&new-version=3.1.4)](https://dependabot.com/compatibility-score.html?dependency-name=cryptiles&package-manager=npm_and_yarn&previous-version=3.1.2&new-version=3.1.4)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot ignore this [patch|minor|major] version` will close this PR and stop Dependabot creating any more for this minor/major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
- `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language
- `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language
- `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language
- `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language
- `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme

Additionally, you can set the following in your Dependabot [dashboard](https://app.dependabot.com):
- Update frequency (including time of day and day of week)
- Automerge options (never/patch/minor, and dev/runtime dependencies)
- Pull request limits (per update run and/or open at any time)
- Out-of-range updates (receive only lockfile updates, if desired)
- Security updates (receive only security updates, if desired)

Finally, you can contact us by mentioning @dependabot.

</details>
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