Skip to content
This repository has been archived by the owner on May 24, 2024. It is now read-only.

Support for executing script from request without any data parameters #12

Merged
merged 2 commits into from
Oct 25, 2017

Conversation

tobiaslindulf
Copy link
Contributor

Fixes #[issue number].

Status

[ ] Under development
[x ] Waiting for code review
[x ] Waiting for merge

Information

[ ] Contains breaking changes
[ ] Contains new API(s)
[ ] Contains documentation
[ ] Contains test
[ ] Contains examples

To-do list

[ ] [Fix this thing]
[ ] [Fix another thing]
[ ] ...

@@ -213,7 +213,10 @@ byte[] GetHeader(Metadata Headers, string Key)
paramnames += $" {param.Name}";
}
logger.Info("{0}", paramnames);
await AddInputData(scriptHeader.Params.ToArray(), requestStream, rserveConn);
if (scriptHeader.Params != null && scriptHeader.Params.Count > 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible for Params to be null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably not since it will be an empty Collection with count 0 if no params are provided. I guess it was a habit to include a null check.

@@ -40,10 +40,6 @@
<HintPath>..\packages\Grpc.Core.1.2.2\lib\net45\Grpc.Core.dll</HintPath>
<Private>True</Private>
</Reference>
<Reference Include="NLog, Version=4.0.0.0, Culture=neutral, PublicKeyToken=5120e14c03d0593c, processorArchitecture=MSIL">
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this include here not needed anymore? As far as I can see NLog is being used at least..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ServerSideExtension project does not use NLog. It is only the SSEtoRserve project that uses it.
NLog was not part of the packages.config but was referenced in the project file for some reason so I cleaned up that warning intentionally.

@@ -40,10 +40,6 @@
<HintPath>..\packages\Grpc.Core.1.2.2\lib\net45\Grpc.Core.dll</HintPath>
<Private>True</Private>
</Reference>
<Reference Include="NLog, Version=4.0.0.0, Culture=neutral, PublicKeyToken=5120e14c03d0593c, processorArchitecture=MSIL">
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this an intentional cleanup?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ServerSideExtension project does not use NLog. It is only the SSEtoRserve project that uses it.
NLog was not part of the packages.config but was referenced in the project file for some reason so I cleaned up that warning intentionally.

@tobiaslindulf tobiaslindulf merged commit 35c0da3 into master Oct 25, 2017
@tobiaslindulf tobiaslindulf deleted the noparam branch October 25, 2017 06:24
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.

None yet

4 participants