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

Add TiledLogs to log list JSON #1635

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mcpherrinm
Copy link

This adds the new TiledLog field to the loglist3 JSON.

It does it in what I think is a minimal backwards-compatible fashion, but which results in duplicating most of the Log struct. As this is a fairly "low level" representation of the log JSON, I think that's an appropriate choice.

There's some other options this could take, so I'd like to hear from the maintainers about how they'd like to proceed here.

We could add the new SubmissionURL and Monitoring URL fields to the existing log struct, and one or the other URLs will be set. Users of this code will have to know how to handle an empty URL, which they probably don't do today.

We could pull all the shared fields into a new "BaseLog" struct, which is embedded in both the TiledLog and Log structs. I think this is a decent option, but is a breaking change.

In any of these situations, I'd like to also update the FindLogBy* methods (or add a new version of each) to support both log types, at least for the shared submission paths which are common between both. That'll require some common representation - perhaps an interface with methods that correctly handle the differences. Supporting reading from a tiled log would be great for this package but is definitely beyond the scope of what I intend to accomplish right now.

@mcpherrinm mcpherrinm requested a review from a team as a code owner December 20, 2024 16:25
@mcpherrinm mcpherrinm requested review from mhutchinson and removed request for a team December 20, 2024 16:25
Copy link

google-cla bot commented Dec 20, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@mcpherrinm
Copy link
Author

force-pushed to change commit email so CLA check passes

@roger2hk roger2hk requested a review from phbnf December 20, 2024 16:48
@roger2hk
Copy link
Contributor

/gcbrun

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