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

[CM-1592] Support for logging of assets folder #39

Merged
merged 64 commits into from
Dec 2, 2021

Conversation

yaricom
Copy link
Member

@yaricom yaricom commented Nov 24, 2021

comet-java-client

  • Added support for the form parameters/fields when uploading files
  • Refined Experiment and OnlineExperiment interfaces to properly handle context
  • Added support of form fields/params for multipart requests
  • Added ApiExperiment interface to make this type of supported experiments
  • Introduced BaseExperimentAsync as superclass for asynchronous experiment implementations
  • Introduced resource bundle to maintain user facing strings
  • Introduced ExperimentContext to maintain context of the experiment: step, epoch, context ID
  • Fixed logCode and uploadAsset to use asynchronous upload operations in OnlineExperiment
  • Implemented logAssetFolder allowing to upload all asset files from specific directory

comet-examples

  • Updated OnlineExperimentExample to include example of assets folder logging
  • Updated OnlineExperimentExample to demonstrate usage of the ExperimentContext
  • Updated OnlineExperimentExample to demonstrate logCode usage

…s prefixed with folder name. Fixed related test cases.
…s prefixed with folder name. Fixed related test cases.
…to indicate whether parent folder name should prefix file names within folder.
…rs step and epoch are null and return the default zero in this case. This is to avoid NullPointerException when invoking getStep()/getEpoch() or nextStep()/nextEpoch().
…pty to properly handle blank strings having only whitespaces.
@codecov-commenter
Copy link

codecov-commenter commented Dec 1, 2021

Codecov Report

Merging #39 (6fdd5b1) into master (8e231d4) will decrease coverage by 0.85%.
The diff coverage is 55.97%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #39      +/-   ##
============================================
- Coverage     53.90%   53.05%   -0.86%     
- Complexity      310      396      +86     
============================================
  Files            55       64       +9     
  Lines          1395     1734     +339     
  Branches        114      139      +25     
============================================
+ Hits            752      920     +168     
- Misses          488      624     +136     
- Partials        155      190      +35     
Flag Coverage Δ
integration 53.05% <55.97%> (-0.86%) ⬇️
unittests 53.05% <55.97%> (-0.86%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...in/java/ml/comet/experiment/ApiExperimentImpl.java 0.00% <0.00%> (ø)
...in/java/ml/comet/experiment/ExperimentBuilder.java 0.00% <0.00%> (ø)
...in/java/ml/comet/experiment/impl/CometApiImpl.java 56.52% <0.00%> (ø)
...java/ml/comet/experiment/impl/asset/AssetType.java 95.00% <ø> (ø)
.../comet/experiment/impl/constants/ApiEndpoints.java 0.00% <ø> (ø)
...et/experiment/impl/http/ConnectionInitializer.java 25.00% <0.00%> (ø)
...ava/ml/comet/experiment/model/LogDataResponse.java 12.50% <0.00%> (-1.79%) ⬇️
.../java/ml/comet/experiment/impl/BaseExperiment.java 30.13% <8.62%> (-25.42%) ⬇️
...ml/comet/experiment/impl/OnlineExperimentImpl.java 38.12% <22.07%> (-9.58%) ⬇️
...l/comet/experiment/impl/resources/LogMessages.java 29.62% <29.62%> (ø)
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e231d4...6fdd5b1. Read the comment docs.

@yaricom
Copy link
Member Author

yaricom commented Dec 1, 2021

Copy link
Member

@Lothiraldan Lothiraldan left a comment

Choose a reason for hiding this comment

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

LGTM, only one small typo

@yaricom yaricom merged commit 0c5cc34 into master Dec 2, 2021
@yaricom yaricom deleted the CM-1592-log-asset-folder branch December 2, 2021 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants