You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "amoeba (via GitHub)" <gi...@apache.org> on 2026/07/29 20:53:37 UTC
[PR] feat(c): add source context to manifest and profile parse errors [arrow-adbc]
amoeba opened a new pull request, #4633:
URL: https://github.com/apache/arrow-adbc/pull/4633
If I have an manifest or profile that's invalid TOML, such as,
```toml
profile_version = 1
driver = not_a_valid_toml_value
```
I get a vague error:
> [Driver Manager] Could not open profile. Error while parsing value: could not determine value type
It turns out the toml++ library can show the line and column number where the parse error occurred. This PR adds that context if it's available so we get a better error:
> [Driver Manager] Could not open profile. Error while parsing value: could not determine value type (line 2, column 10).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(c): add source context to manifest and profile parse errors [arrow-adbc]
Posted by "amoeba (via GitHub)" <gi...@apache.org>.
amoeba commented on PR #4633:
URL: https://github.com/apache/arrow-adbc/pull/4633#issuecomment-5124895103
Should be good now. 🤞
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(c): add source context to manifest and profile parse errors [arrow-adbc]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #4633:
URL: https://github.com/apache/arrow-adbc/pull/4633#discussion_r3678907689
##########
c/driver_manager/adbc_driver_manager_test.cc:
##########
@@ -2059,6 +2059,46 @@ TEST_F(ConnectionProfiles, CustomProfileProvider) {
ASSERT_THAT(AdbcDatabaseRelease(&database.value, &error), IsOkStatus(&error));
}
+TEST_F(ConnectionProfiles, ProfileParseErrorIncludesLocation) {
+ auto filepath = temp_dir / "badprofile.toml";
+ std::ofstream file(filepath);
+ ASSERT_TRUE(file.is_open());
+ file << "profile_version = 1\n"
+ << "driver = unquoted_value\n"
+ << "[Options]\n";
+ file.close();
+
+ adbc_validation::Handle<struct AdbcDatabase> database;
+ ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "profile", filepath.string().c_str(),
+ &error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseInit(&database.value, &error),
+ IsStatus(ADBC_STATUS_INVALID_ARGUMENT, &error));
+ ASSERT_THAT(error.message, ::testing::HasSubstr("line 2"));
+ ASSERT_THAT(error.message, ::testing::HasSubstr("column"));
+ ASSERT_THAT(AdbcDatabaseRelease(&database.value, &error), IsOkStatus(&error));
+}
+
+TEST_F(DriverManifest, ManifestParseErrorIncludesLocation) {
+ std::ofstream file(temp_dir / "baddriver.toml");
+ ASSERT_TRUE(file.is_open());
+ file << "manifest_version = 1\n"
+ << "[Driver]\n"
+ << "shared = unquoted_value\n";
+ file.close();
+
+ SetDriverPath(temp_dir.string().c_str());
+
+ ASSERT_THAT(AdbcFindLoadDriver("baddriver", nullptr, ADBC_VERSION_1_1_0,
+ ADBC_LOAD_FLAG_DEFAULT, nullptr, &driver, &error),
+ Not(IsOkStatus(&error)));
Review Comment:
nit: can't you directly use IsStatus?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(c): add source context to manifest and profile parse errors [arrow-adbc]
Posted by "amoeba (via GitHub)" <gi...@apache.org>.
amoeba commented on PR #4633:
URL: https://github.com/apache/arrow-adbc/pull/4633#issuecomment-5147806313
Merged, thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(c): add source context to manifest and profile parse errors [arrow-adbc]
Posted by "amoeba (via GitHub)" <gi...@apache.org>.
amoeba commented on code in PR #4633:
URL: https://github.com/apache/arrow-adbc/pull/4633#discussion_r3678981000
##########
c/driver_manager/adbc_driver_manager_test.cc:
##########
@@ -2059,6 +2059,46 @@ TEST_F(ConnectionProfiles, CustomProfileProvider) {
ASSERT_THAT(AdbcDatabaseRelease(&database.value, &error), IsOkStatus(&error));
}
+TEST_F(ConnectionProfiles, ProfileParseErrorIncludesLocation) {
+ auto filepath = temp_dir / "badprofile.toml";
+ std::ofstream file(filepath);
+ ASSERT_TRUE(file.is_open());
+ file << "profile_version = 1\n"
+ << "driver = unquoted_value\n"
+ << "[Options]\n";
+ file.close();
+
+ adbc_validation::Handle<struct AdbcDatabase> database;
+ ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "profile", filepath.string().c_str(),
+ &error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseInit(&database.value, &error),
+ IsStatus(ADBC_STATUS_INVALID_ARGUMENT, &error));
+ ASSERT_THAT(error.message, ::testing::HasSubstr("line 2"));
+ ASSERT_THAT(error.message, ::testing::HasSubstr("column"));
+ ASSERT_THAT(AdbcDatabaseRelease(&database.value, &error), IsOkStatus(&error));
+}
+
+TEST_F(DriverManifest, ManifestParseErrorIncludesLocation) {
+ std::ofstream file(temp_dir / "baddriver.toml");
+ ASSERT_TRUE(file.is_open());
+ file << "manifest_version = 1\n"
+ << "[Driver]\n"
+ << "shared = unquoted_value\n";
+ file.close();
+
+ SetDriverPath(temp_dir.string().c_str());
+
+ ASSERT_THAT(AdbcFindLoadDriver("baddriver", nullptr, ADBC_VERSION_1_1_0,
+ ADBC_LOAD_FLAG_DEFAULT, nullptr, &driver, &error),
+ Not(IsOkStatus(&error)));
Review Comment:
Yeah, that's better. Done in c68a49cee286d7abccee6e5ba7e7be104859c3ac.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(c): add source context to manifest and profile parse errors [arrow-adbc]
Posted by "amoeba (via GitHub)" <gi...@apache.org>.
amoeba merged PR #4633:
URL: https://github.com/apache/arrow-adbc/pull/4633
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org