Skip to content

Remove the KIDL async keyword #61

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

Merged
merged 2 commits into from
Mar 24, 2025
Merged

Remove the KIDL async keyword #61

merged 2 commits into from
Mar 24, 2025

Conversation

MrCreosote
Copy link
Member

@MrCreosote MrCreosote commented Mar 22, 2025

Replaced years ago by specifying async status on the command line when compiling or automatically getting it from the catalog for kb-sdk install

Note there's some whitespace changes

Replaced years ago by specifying async status on the command line when
compiling or automatically getting it from the catalog for kb-sdk
install
@MrCreosote MrCreosote requested a review from briehl March 22, 2025 01:11
Copy link
Member Author

Choose a reason for hiding this comment

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

Everything in this folder except for SpecParser.jj is generated by javacc

boolean anyAsync = asyncVersion != null || anyAsync(module);
boolean anyAsync = asyncVersion != null;
Copy link
Member Author

Choose a reason for hiding this comment

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

This method is 500 or so lines long, so the next 3 anyAsync bools are coming from here

Copy link

codecov bot commented Mar 22, 2025

Codecov Report

Attention: Patch coverage is 64.22018% with 39 lines in your changes missing coverage. Please review.

Project coverage is 65.43%. Comparing base (94ca75c) to head (63072e8).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
...rc/java/us/kbase/jkidl/SpecParserTokenManager.java 45.83% 18 Missing and 8 partials ⚠️
src/java/us/kbase/jkidl/SpecParser.java 75.00% 5 Missing and 2 partials ⚠️
...java/us/kbase/mobu/compiler/JavaTypeGenerator.java 80.64% 3 Missing and 3 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main      #61      +/-   ##
============================================
- Coverage     65.53%   65.43%   -0.11%     
+ Complexity     1424     1401      -23     
============================================
  Files            77       77              
  Lines          7005     6940      -65     
  Branches       1300     1283      -17     
============================================
- Hits           4591     4541      -50     
+ Misses         1931     1921      -10     
+ Partials        483      478       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment on lines -132 to -151
if (asyncByDefault) {
// Make all methods async and mark methods being async already as couldn't be sync
context.put("any_async", true);
List<Map<String, Object>> modules = (List<Map<String, Object>>)context.get("modules");
for (int modulePos = 0; modulePos < modules.size(); modulePos++) {
Map<String, Object> module = modules.get(modulePos);
module.put("any_async", true);
List<Map<String, Object>> methods = (List<Map<String, Object>>)module.get("methods");
if (methods == null)
continue;
for (int methodPos = 0; methodPos < methods.size(); methodPos++) {
Map<String, Object> method = methods.get(methodPos);
//TODO ROMAN should async go into the map or can this line be deleted?
// Boolean async = (Boolean)method.get("async");
method.put("async", true);
//if (async == null || !async)
// method.put("could_be_sync", true);
}
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This section has no effect, since none of the keys it adds to the map are used in the python server or impl files. Not sure about the other languages but they're being removed anyway

@@ -104,7 +103,6 @@ public static void generate(List<KbService> srvs, String defaultUrl,
jsClient.close();
}
Map<String, Object> perlMakefileContext = new LinkedHashMap<String, Object>(context);
Map<String, Object> pyMakefileContext = new LinkedHashMap<String, Object>(context);
Copy link
Member Author

Choose a reason for hiding this comment

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

Cruft I missed on an earlier PR

async, clientAsyncVer, dynservVer, semanticVersion, gitUrl, gitCommitHash);
clientAsyncVer, dynservVer, semanticVersion, gitUrl, gitCommitHash);
Copy link
Member Author

Choose a reason for hiding this comment

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

If async is true at this point clientAsyncVer is guaranteed not to be null

Copy link
Member

@briehl briehl left a comment

Choose a reason for hiding this comment

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

👍

@MrCreosote MrCreosote merged commit 193e877 into main Mar 24, 2025
9 of 11 checks passed
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.

2 participants