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.

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

Fix #23: Expose application_name to magic #24

Merged
merged 3 commits into from
Mar 23, 2017
Merged

Fix #23: Expose application_name to magic #24

merged 3 commits into from
Mar 23, 2017

Conversation

patrick-nicholson
Copy link
Contributor

Addresses #23

@codecov
Copy link

codecov bot commented Mar 23, 2017 •
edited
Loading

Codecov Report

Merging #24 into master will increase coverage by 0.16%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #24      +/-   ##
==========================================
+ Coverage   85.31%   85.48%   +0.16%     
==========================================
  Files           6        6              
  Lines         429      434       +5     
==========================================
+ Hits          366      371       +5     
  Misses         63       63

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c9267b1...412db21. Read the comment docs.

parente
parente reviewed Mar 23, 2017
View reviewed changes
spylon_kernel/init_spark_magic.py Outdated
@@ -37,8 +39,9 @@ def cell_init_spark(self):

globals_dict = self.env
exec(self.code, globals_dict)
application_name = globals_dict.get('application_name', None)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm looking at a diff quickly, but I'm guessing you could just put "ScalaMetaKernel" here in place of the None and remove the check later. Also, it should probably be "spylon-kernel" by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about a global DEFAULT_APPLICATION_NAME in scala_interpreter.py that can be used in both files? I just used "ScalaMetaKernel" as it's the init_spark_session default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since I instantiate it as None, get is never actually going to return the default. Removing it for clarity.

@parente parente merged commit e1ed9b4 into vericast:master Mar 23, 2017
@parente
Copy link
Contributor

parente commented Mar 23, 2017

🍰

@parente parente mentioned this pull request Mar 23, 2017
Closed
@patrick-nicholson patrick-nicholson changed the title Added application name to magic Fix #23: Expose application_name to magic Mar 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@parente parente parente left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@patrick-nicholson @parente