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

Remove pointer to NC from open/create in the dispatch table #1450

Merged
merged 21 commits into from
Aug 5, 2019
Merged

Remove pointer to NC from open/create in the dispatch table #1450

merged 21 commits into from
Aug 5, 2019

Conversation

edwardhartnett
Copy link
Contributor

Part of #1426

In this PR I remove pointer to NC from open/create in the dispatch table, and all dispatch layers.

@DennisHeimbigner note the changes in libdap2/libdap4. I have copied the local coding style.

This allows us to provide the dispatch tables without exposing the NC struct. Now, only primitive C types, and a pointer to the dispatch table itself, are found in the dispatch table.

This was quite easy and simple. The dispatch layer creates the ncid (which we sometimes call ext_ncid). Some layers (like the classic netcdf, and pnetcdf) can also use the int_ncid internally. This PR does not touch anything to do with the int_ncid.

This PR includes pending PR #441

This PR also turns off the currently failing tst_zero_len_var.sh in both autotools and cmake builds (see #1449). Once that issue is resolved I can turn that test back on.

@WardF
Copy link
Member

WardF commented Aug 1, 2019

Just FYI, the opendap test server appears to be back up and running as of a few moments ago.

@WardF WardF self-assigned this Aug 1, 2019
@WardF WardF added this to the 4.7.1 milestone Aug 1, 2019
@edwardhartnett
Copy link
Contributor Author

Did a little more refinement to clean up the code.

@WardF thanks for marking this for 4.7.1. These changes will be necessary for PIO integration to work. They also enable much fuller use of the user defined format feature.

@edwardhartnett
Copy link
Contributor Author

@WardF maybe you could relaunch this? The dap test servers seem to be a bit wonky today...

@edwardhartnett
Copy link
Contributor Author

OK, various failures in different jobs each time in ncdap testing. Something is wonky with the server, or else timeouts need to be increased.

Also, @WardF, I suggest you add --disable-dap to some of the travis tests, so the servers don't get so hammered each time.

@WardF WardF merged commit d1ddc92 into Unidata:master Aug 5, 2019
@edhartnett edhartnett deleted the ejh_remove_nc branch August 13, 2019 17:12
@DennisHeimbigner
Copy link
Collaborator

I really wish you had not done this without more discussion.

@WardF
Copy link
Member

WardF commented Aug 15, 2019

@DennisHeimbigner Is there an issue? We can take a look and address issues now.

@edhartnett
Copy link
Contributor

Without this, user-defined format users would have to see the contents of the NC struct.

@DennisHeimbigner
Copy link
Collaborator

I understand the need for something like this. Well, it is too late now.
It is simpler to move forward with this change in place.

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.

4 participants