Skip to content

Dataset Management API Support#83

Open
PeterAustinMoore wants to merge 8 commits into
CityofSantaMonica:masterfrom
PeterAustinMoore:master
Open

Dataset Management API Support#83
PeterAustinMoore wants to merge 8 commits into
CityofSantaMonica:masterfrom
PeterAustinMoore:master

Conversation

@PeterAustinMoore
Copy link
Copy Markdown

This is my first foray into C# and .NET, so any pointers/feedback greatly appreciated. I tried to set up the building blocks based off of https://github.com/socrata/socrata-py

What it adds:

  • CreateDataset method through DSMAPI
  • CreateRevision off of a currently existing dataset
  • Error Rows for catching problematic data

What it removes:

  • Nothing

What it could do in the future:

  • Column level schema changes on dataset creation/revision
  • Using Config (similar to socrata-py)
  • Metadata only dataset updates
  • Create revision based off external link, the current dataset, or a Gateway

@JoeNunnelley
Copy link
Copy Markdown

General : Run FxCop on this code base (or some more modern analog)
https://docs.microsoft.com/en-us/previous-versions/dotnet/netframework-3.0/bb429476(v=vs.80)?redirectedfrom=MSDN
also run
https://github.com/StyleCop

File : SODA.Tests/SodcaClientTests.cs

  • From the readme it looks like we are renaming DSMAPI intannces to SODAClient. Here we are naming tests with DSMAPI in them and I wonder if we should convert to using SODAClient throughout?
    File : SODA/SodaClient.cs
  • Some instances where you use str == null to check values when its probably ok to use the IsNullOrEmpty() function as you do in other locations

File : README.md

  • you can use this formulation to avoid all the extra escape chars in your example code : string path1 = @"c:\temp\MyTest.txt";

File : SODA/PipelineJob.cs

  • public string Username => publis string username to match other variable use in the files.

Copy link
Copy Markdown

@JoeNunnelley JoeNunnelley left a comment

Choose a reason for hiding this comment

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

here are some suggestions. probably the most valuable will be to run static code analysis tools and style tools on this project.

Comment thread SODA/PipelineJob.cs Outdated
Comment thread SODA/PipelineJob.cs Outdated
Comment thread SODA/SchemaTransforms.cs Outdated
Comment thread SODA/SodaClient.cs
/// <exception cref="System.ArgumentOutOfRangeException">Thrown if the specified <paramref name="resourceId"/> does not match the Socrata 4x4 pattern.</exception>
public Revision CreateRevision(string method, string resourceId, string permission = "private")
{

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

extra line

Comment thread SODA/SodaClient.cs
Comment thread SODA/Utilities/SodaUri.cs Outdated
@thekaveman
Copy link
Copy Markdown
Contributor

@PeterAustinMoore this is a tremendous effort, thank you for submitting the PR!

I haven't had the time to fully dig in and review yet, but I wanted to let you know its on our radar. One thing that would greatly help the review is if you could rebase your branch on master (we had some in-flight changes that were just released yesterday).

I can't promise when we'll get to this, but we are definitely interested in folding in the functionality. Stay tuned!

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