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

Allowing relative script file references #1132

Merged
merged 1 commit into from
Jan 21, 2017
Merged

Conversation

fabiocav
Copy link
Member

@fabiocav fabiocav commented Jan 18, 2017

This enables relative references to script files, primarily addressing the need to reference shared precompiled assemblies, but enabling this for other languages as well.

With this changes, relative references like this are now supported:
"scriptFile":"..\\shared\\myassembly.dll"

@fabiocav fabiocav changed the title [WIP] Allowing relative script file references Allowing relative script file references Jan 19, 2017
@fabiocav fabiocav changed the title Allowing relative script file references [WIP] Allowing relative script file references Jan 19, 2017
@mamaso
Copy link
Contributor

mamaso commented Jan 19, 2017

Looks good! How will this interact with script host file watching?

@fabiocav
Copy link
Member Author

fabiocav commented Jan 19, 2017

Good question! This would currently rely on watchDirectories configuration, which is consistent with the way the runtime deals with other shared scripts (non-primary script files).
We could be smarter about this, since we can detect those dependencies.
Some of this (for precompiled functions) is being addressed in some work in progress for shadow copying and runtime restart when assemblies are updated.

@fabiocav fabiocav changed the title [WIP] Allowing relative script file references Allowing relative script file references Jan 19, 2017
@fabiocav fabiocav requested a review from mathewc January 19, 2017 18:45
string scriptPath = fileSystem.Path.Combine(scriptDirectory, scriptFileName);
if (!fileSystem.File.Exists(scriptPath))
{
throw new ConfigurationErrorsException("Invalid script file name configuration. The 'scriptFileName' property is set to a file that does not exist.");
Copy link
Member

Choose a reason for hiding this comment

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

The property is called "scriptFile" right?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe rename local var to scriptFile, since it can actually be a path.

@@ -0,0 +1,16 @@
{
"scriptFile": "../HttpTrigger/index.js",
Copy link
Member

Choose a reason for hiding this comment

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

You used \ in a previous example, here you're using /. Intentional?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was.... just wanted to have a sample and test with the variation

Copy link
Member

@mathewc mathewc left a comment

Choose a reason for hiding this comment

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

One minor comment

@fabiocav fabiocav force-pushed the relscriptfile branch 2 times, most recently from f92c060 to b9f6750 Compare January 20, 2017 21:37
@fabiocav fabiocav merged commit 7114217 into Azure:dev Jan 21, 2017
@fabiocav fabiocav deleted the relscriptfile branch January 21, 2017 01:15
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