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

feat: upload user files #216

Merged
merged 35 commits into from
Mar 22, 2025
Merged

Conversation

greatgitsby
Copy link
Contributor

@greatgitsby greatgitsby commented Mar 22, 2025

closes #51

keeping in mind the LoC limits, minimized some lines in nearby APIs

Copy link

github-actions bot commented Mar 22, 2025

deployed preview: https://216.connect-d5y.pages.dev

Welcome to connect! Make sure to:

  • read the contributing guidelines
  • mark your PR as a draft until it's ready to review
  • post the preview on Discord; feedback from users will speedup the PR review

Mobile

Desktop

@greatgitsby greatgitsby marked this pull request as ready for review March 22, 2025 02:19
@greatgitsby greatgitsby changed the title feat: implement some of the athenad + comma calls for managing user files feat: implement several athenad + comma calls for managing user files Mar 22, 2025
@incognitojam incognitojam added the enhancement new feature or request label Mar 22, 2025
@greatgitsby
Copy link
Contributor Author

we should have a backlog item for defining style in Biome to ensure things like line wrapping and quotes are consistent

not super big deal, but it'll help avoid you having to make small nit commits on everyone's PRs 😄

@greatgitsby
Copy link
Contributor Author

#220

@incognitojam
Copy link
Member

Not counting blank lines in the lint count means we don't need to be super aggressive anymore 🙂

Copy link
Member

@incognitojam incognitojam left a comment

Choose a reason for hiding this comment

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

We should have the "upload all segments" button in this PR so that is isn't just adding unused code 😄

This PR stills adds lots of methods and types for the upload queue. Normally I would try to split PRs along the line of features rather than "backend" and UI like this, but I'm fine with leaving this stuff here in this case if you want to

The next PR should add the upload queue

@greatgitsby
Copy link
Contributor Author

sounds great!

@greatgitsby greatgitsby changed the title feat: implement several athenad + comma calls for managing user files feat: managing user files Mar 22, 2025
@greatgitsby greatgitsby changed the title feat: managing user files feat: upload user files Mar 22, 2025
Copy link
Member

@incognitojam incognitojam left a comment

Choose a reason for hiding this comment

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

Just style changes 😅

Will try and get the Biome formatter setup

@incognitojam incognitojam merged commit fe511dc into commaai:master Mar 22, 2025
4 checks passed
@greatgitsby greatgitsby deleted the feat/upload-apis branch March 22, 2025 22:49
const fileTypesToUpload = buttonToFileTypeMap[type]

try {
await uploadAllSegments(props.routeName, route.segment_numbers.length, fileTypesToUpload)
Copy link
Contributor

@sshane sshane Mar 29, 2025

Choose a reason for hiding this comment

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

We only added routes_segments to get maxqlog? We're trying to deprecate it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is this written in documentation somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

happy to change it

Copy link
Contributor

Choose a reason for hiding this comment

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

d["segment_numbers"] = list(range(0, r.maxqlog+1))

we can just use this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'll patch this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sshane added a commit that referenced this pull request Mar 29, 2025
Added in #216

from `routes_segments` API:

```
    for row in q.all():
      d["start_time_utc_millis"] = int((0 if r.start_time is None else r.start_time.timestamp()) * 1000)
      d["end_time_utc_millis"] = int((0 if r.end_time is None else r.end_time.timestamp()) * 1000)

      # TODO: remove these once connect doesn't use them
      d["segment_numbers"] = list(range(0, r.maxqlog+1))
      d["segment_start_times"] = [d["start_time_utc_millis"]+(i*60*1000) for i in range(r.maxqlog+1)]
      d["segment_end_times"] = [min(d["segment_start_times"][i]+(60*1000), d["end_time_utc_millis"]) for i in range(r.maxqlog+1)]

      res[r.fullname] = d

  return jsonify(list(res.values()))
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement file uploads
3 participants