Skip to content

Conversation

@hadim
Copy link
Contributor

@hadim hadim commented Apr 23, 2017

  • Similar to the Groovy grab mechanism.
  • Used in the scijava-jupyter-kernel.

@hadim
Copy link
Contributor Author

hadim commented Apr 23, 2017

It's working :

# @Grab(group="org.springframework", module="spring-orm", version="3.2.5.RELEASE")

from org.springframework.jdbc.core import JdbcTemplate
print(JdbcTemplate)

@ctrueden
Copy link
Member

This is great!

One thing I'd like to discuss, since you needed to implement this feature yourself anyway: can we enrich it to show better progress relating to what's happening? The Beaker-based Grape support would simply hang until all dependency resolution was complete. And if something went wrong, there was little/no indication of what. There is as particular problematic artifact in the ImageJ2 dependency hierarchy—asm-tree IIRC?—which caused the bootstrapping to fail on many users' machines. They needed to manually copy this artifact from their ~/.m2/repository into ~/.groovy/grapes, and then rerun the notebook. It would of course be much nicer if the bootstrapping was more resilient and informative than that.

@hadim
Copy link
Contributor Author

hadim commented Apr 24, 2017

I am reusing the groovy.grape classes so I didn't have to reimplement myself everything.

The hard work is done by GrapeIvy which is coded in Groovy.

I first tried to fork the class and convert it to Java but there it too much Groovy-specific codes so I had to give up. I also add to extend the class because of a ClassLoader check that I had to overwrite.

For now, I am using log.debug to display something when the kernel starts grabbing an artifact. I could easily switch it to log.info. But for the progress, I am afraid the only solution would be to rewrite this GrapeIvy class.

Now, I can definitely consider doing that in the future but for now, I'd like to let it like that in order to release a first version of the kernel as fast as possible.

Unless you see another easier solution of course.

@ctrueden
Copy link
Member

ctrueden commented Apr 24, 2017

Interesting; there is a System.err.println call in there. I wonder why we aren't seeing that?

Unless you see another easier solution of course.

Might it be doable to monitor (poll) the GrapeIvy instance fields resolvedDependencies and downloadedArtifacts, together with the grabRecordsForCurrDependencies, to get a feel for how far along the process is?

@hadim
Copy link
Contributor Author

hadim commented Apr 24, 2017

I just did a quick test and an error if returned and displayed to the user if something went wrong (for example bad group or artifact id).

@hadim
Copy link
Contributor Author

hadim commented Apr 24, 2017

What do you mean monitoring instance fields? I don' understand how to do that.

@hadim
Copy link
Contributor Author

hadim commented Apr 25, 2017

Well, Ivy has an option to report progress on downloads. I have enabled it in my last commit.

# @Grab(group="org.springframework", module="spring-orm", version="3.2.5.RELEASE")
from org.springframework.jdbc.core import JdbcTemplate
Downloaded 3409 Kbytes in 3079ms:
  [SUCCESSFUL ] org.scijava#scijava-common;2.62.1!scijava-common.jar (10ms)
  [SUCCESSFUL ] org.scijava#parsington;1.0.0!parsington.jar (1ms)
  [SUCCESSFUL ] com.googlecode.gentyref#gentyref;1.1.0!gentyref.jar (1ms)
  [SUCCESSFUL ] org.bushe#eventbus;1.4!eventbus.jar (1ms)
  [SUCCESSFUL ] org.springframework#spring-orm;3.2.5.RELEASE!spring-orm.jar (633ms)
  [SUCCESSFUL ] aopalliance#aopalliance;1.0!aopalliance.jar (1ms)
  [SUCCESSFUL ] org.springframework#spring-beans;3.2.5.RELEASE!spring-beans.jar (704ms)
  [SUCCESSFUL ] org.springframework#spring-core;3.2.5.RELEASE!spring-core.jar (667ms)
  [SUCCESSFUL ] commons-logging#commons-logging;1.1.1!commons-logging.jar (1ms)
  [SUCCESSFUL ] org.springframework#spring-jdbc;3.2.5.RELEASE!spring-jdbc.jar (577ms)
  [SUCCESSFUL ] org.springframework#spring-tx;3.2.5.RELEASE!spring-tx.jar (476ms)

@hadim
Copy link
Contributor Author

hadim commented Apr 25, 2017

(it's not real time but at least we have some feedback on what's happening)

@hadim
Copy link
Contributor Author

hadim commented Apr 25, 2017

I think I am done here.

You can review and/org merge when you want.

@hadim
Copy link
Contributor Author

hadim commented Apr 25, 2017

(Also I just checked and the mechanism works well with the Fiji script editor)

@ctrueden
Copy link
Member

ctrueden commented May 6, 2017

I'm trying to decide how best to introduce this feature. I do not want to add another dependency to SciJava Common, so we may need a new component. I'll consider more over the weekend.

@hadim
Copy link
Contributor Author

hadim commented May 6, 2017

We could consider copy the required code from the Groovy repo but one of the class is coded in Groovy.

@ctrueden
Copy link
Member

ctrueden commented May 16, 2017

I have some questions about this grabbing feature. It seems to me that a different syntax here would be superior to Groovy's @Grab and @GrabResolver. Because IIUC, those annotations do not actually alter the import statement, but rather simply tell the Groovy interpreter to obtain those artifacts prior to executing any code in the script.

So, with that in mind, why don't we instead support something like this:

#!groovy
#@repository("https://maven.imagej.net/content/groups/public")
#@dependency("net.imagej:imagej:2.0.0-rc-60")
import net.imagej.ImageJ
ij = new ImageJ()
ij.log().info("Hello world")

This syntax would then be independent of all languages, and simply be another "preprocessing" or preparation step prior to script execution. I can implement some kind of general purpose script preprocessor which handles these preprocessing directives, and then we can create a new component scijava-dependencies which adds a DependencyService as well as a script preprocessor that uses it.

Does this sound like a sane plan? I do not feel strongly about the exact syntax; for example, if someone has a better suggestion than #@dependency, that's OK.

@hadim @imagejan What do you think?

@hadim
Copy link
Contributor Author

hadim commented May 16, 2017

I am good with that. And re-using the shebang character # seems like a good idea.

@ctrueden
Copy link
Member

Awesome. I will work on this. I can also finally add a #@script which is equivalent to the Java @Plugin annotation, so the script can declare the same metadata as plugins, such as iconPath.

@imagejan
Copy link
Member

imagejan commented May 17, 2017

Sounds great!
I wonder whether it might be confusing for people who are used to use @Grab in groovy scripts. Will it be compatible with this, i.e. can you do the following:

#!groovy
#@dependency("org.codehaus.groovy:groovy")
// @LogService log

@Grab('commons-lang:commons-lang:2.4')

// ... do whatever with log and commons-lang

Although the #@dependency syntax (a combination of #! shebang and scijava parameters) seems a bit uncommon to me, I have no better suggestion. It might however lead to confusions when someone mixes capitalization, spaces, and symbols like this:

#!Groovy
# @Dependency
// @imagej

Oh, and syntax highlighting is another issue ;-)

@ctrueden
Copy link
Member

ctrueden commented May 17, 2017

@imagejan wrote:

the #@dependency syntax (a combination of #! shebang and scijava parameters) seems a bit uncommon to me

That is why I like it: it is unlikely to collide with syntax from any of the script languages themselves.

My plan is to replace all #@ "preprocessor" lines with blank lines before feeding the script to the JSR-223 engine. That way, the line numbers will still be correct, and we can also use this mechanism for the script parameters themselves:

#@input String name
#@input int age
#@output String greeting

I will keep support for the old comment-based syntax, for backwards compatibility. But this new syntax will work throughout the script.

There will be a "catch-all" preprocessor for #@unknownKeyword foo bar type things, which will default to behaving like #@input, in the same way that the // @ syntax does now.

Do you see any problems with this scheme?

It might however lead to confusions when someone mixes capitalization, spaces, and symbols

We'll be permissive in what we accept—e.g., case will not matter—but the documentation will use small case by convention, I think.

@hadim
Copy link
Contributor Author

hadim commented May 17, 2017

Good for me.

ctrueden added a commit that referenced this pull request May 22, 2017
This will be useful for implementing script processing directives in
an extensible way. Here is one example, for dependency declaration:

  #@repository('https://maven.imagej.net/content/groups/public')
  #@dependency('net.imagej:imagej:2.0.0-rc-60')

And here is another, for defining the intended script language:

  #!clojure

We will also migrate the script parameter syntax to this scheme:

  #@input String name
  #@input int age
  #@output String greeting

Or with the familiar sloppy shorthand:

  #@string name
  #@int age
  #@output String greeting

See #265
See scijava/scijava-jupyter-kernel#51 (comment)
ctrueden added a commit that referenced this pull request May 24, 2017
This will be useful for implementing script processing directives in
an extensible way. Here is one example, for dependency declaration:

  #@repository('https://maven.imagej.net/content/groups/public')
  #@dependency('net.imagej:imagej:2.0.0-rc-60')

And here is another, for defining the intended script language:

  #!clojure

We will also migrate the script parameter syntax to this scheme:

  #@input String name
  #@input int age
  #@output String greeting

Or with the familiar sloppy shorthand:

  #@string name
  #@int age
  #@output String greeting

See #265
See scijava/scijava-jupyter-kernel#51 (comment)
ctrueden added a commit that referenced this pull request May 24, 2017
ctrueden pushed a commit to scijava/scijava-grab that referenced this pull request May 24, 2017
See scijava/scijava-common#265.

Signed-off-by: Curtis Rueden <ctrueden@wisc.edu>
ctrueden added a commit that referenced this pull request May 24, 2017
This will be useful for implementing script processing directives in
an extensible way. Here is one example, for dependency declaration:

  #@repository('https://maven.imagej.net/content/groups/public')
  #@dependency('net.imagej:imagej:2.0.0-rc-60')

And here is another, for defining the intended script language:

  #!clojure

We will also migrate the script parameter syntax to this scheme:

  #@input String name
  #@input int age
  #@output String greeting

Or with the familiar sloppy shorthand:

  #@string name
  #@int age
  #@output String greeting

See #265
See scijava/scijava-jupyter-kernel#51 (comment)
ctrueden added a commit that referenced this pull request May 24, 2017
@ctrueden
Copy link
Member

This PR has been used as the basis for a new repository scijava/scijava-grab. Closing this PR in favor of that repository.

@ctrueden ctrueden closed this May 24, 2017
ctrueden added a commit that referenced this pull request May 24, 2017
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.

3 participants